| Message ID | 20190730172508.19911-6-mreitz@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qcow2: Let check -r all repair some snapshot bits | expand |
On 7/30/19 12:25 PM, Max Reitz wrote: > qcow2 v3 requires every snapshot table entry to have two extra data > fields: The 64-bit VM state size, and the virtual disk size. Both are > optional for v2 images, so they may not be present. > > qcow2_upgrade() therefore should update the snapshot table to ensure all > entries have these extra data fields. > > Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1727347 > Reported-by: Eric Blake <eblake@redhat.com> > Signed-off-by: Max Reitz <mreitz@redhat.com> > --- > block/qcow2.c | 29 +++++++++++++++++++++++++++-- > 1 file changed, 27 insertions(+), 2 deletions(-) > > + > + /* > + * In v2, snapshots do not need to have extra data. v3 requires > + * the 64-bit VM state size and the virtual disk size to be > + * present. > + * qcow2_write_snapshots() will always write the list in the > + * v3-compliant format. > + */ > + need_snapshot_update = false; > + for (i = 0; i < s->nb_snapshots; i++) { > + if (s->snapshots[i].extra_data_size < 16) { s/16/sizeof(extra)/ looks a bit nicer, but doesn't change semantics. Reviewed-by: Eric Blake <eblake@redhat.com>
On 30.07.19 20:10, Eric Blake wrote: > On 7/30/19 12:25 PM, Max Reitz wrote: >> qcow2 v3 requires every snapshot table entry to have two extra data >> fields: The 64-bit VM state size, and the virtual disk size. Both are >> optional for v2 images, so they may not be present. >> >> qcow2_upgrade() therefore should update the snapshot table to ensure all >> entries have these extra data fields. >> >> Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1727347 >> Reported-by: Eric Blake <eblake@redhat.com> >> Signed-off-by: Max Reitz <mreitz@redhat.com> >> --- >> block/qcow2.c | 29 +++++++++++++++++++++++++++-- >> 1 file changed, 27 insertions(+), 2 deletions(-) >> > >> + >> + /* >> + * In v2, snapshots do not need to have extra data. v3 requires >> + * the 64-bit VM state size and the virtual disk size to be >> + * present. >> + * qcow2_write_snapshots() will always write the list in the >> + * v3-compliant format. >> + */ >> + need_snapshot_update = false; >> + for (i = 0; i < s->nb_snapshots; i++) { >> + if (s->snapshots[i].extra_data_size < 16) { > > s/16/sizeof(extra)/ looks a bit nicer, but doesn't change semantics. Hm, but it’s not quite right. I mean, right now it is, but if we were to add a new field to snapshot metadata, it wouldn’t be. It should be 16. I can make it something like sizeof(extra.vm_state_size_large) + sizeof(extra.disk_size), though. Max > Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/block/qcow2.c b/block/qcow2.c index 99e9dad798..75d41683a1 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -4731,7 +4731,9 @@ static int qcow2_upgrade(BlockDriverState *bs, int target_version, Error **errp) { BDRVQcow2State *s = bs->opaque; + bool need_snapshot_update; int current_version = s->qcow_version; + int i; int ret; /* This is qcow2_upgrade(), not qcow2_downgrade() */ @@ -4740,7 +4742,30 @@ static int qcow2_upgrade(BlockDriverState *bs, int target_version, /* There are no other versions (yet) that you can upgrade to */ assert(target_version == 3); - status_cb(bs, 0, 1, cb_opaque); + status_cb(bs, 0, 2, cb_opaque); + + /* + * In v2, snapshots do not need to have extra data. v3 requires + * the 64-bit VM state size and the virtual disk size to be + * present. + * qcow2_write_snapshots() will always write the list in the + * v3-compliant format. + */ + need_snapshot_update = false; + for (i = 0; i < s->nb_snapshots; i++) { + if (s->snapshots[i].extra_data_size < 16) { + need_snapshot_update = true; + break; + } + } + if (need_snapshot_update) { + ret = qcow2_write_snapshots(bs); + if (ret < 0) { + error_setg_errno(errp, -ret, "Failed to update the snapshot table"); + return ret; + } + } + status_cb(bs, 1, 2, cb_opaque); s->qcow_version = target_version; ret = qcow2_update_header(bs); @@ -4749,7 +4774,7 @@ static int qcow2_upgrade(BlockDriverState *bs, int target_version, error_setg_errno(errp, -ret, "Failed to update the image header"); return ret; } - status_cb(bs, 1, 1, cb_opaque); + status_cb(bs, 2, 2, cb_opaque); return 0; }
qcow2 v3 requires every snapshot table entry to have two extra data fields: The 64-bit VM state size, and the virtual disk size. Both are optional for v2 images, so they may not be present. qcow2_upgrade() therefore should update the snapshot table to ensure all entries have these extra data fields. Buglink: https://bugzilla.redhat.com/show_bug.cgi?id=1727347 Reported-by: Eric Blake <eblake@redhat.com> Signed-off-by: Max Reitz <mreitz@redhat.com> --- block/qcow2.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)