Actually test that the full range is available. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; --- tests/qemublocktest.c | 4 ++-- tests/qemublocktestdata/imagecreate/luks-encopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/luks-noopts.json | 4 ++-- .../qemublocktestdata/imagecreate/network-gluster-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json | 4 ++-- .../qemublocktestdata/imagecreate/network-sheepdog-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json | 4 ++-- .../qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json | 4 ++-- .../imagecreate/qcow2-luks-encopts-backing.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json | 4 ++-- tests/qemublocktestdata/imagecreate/qcow2.json | 4 ++-- tests/qemublocktestdata/imagecreate/raw.json | 2 +- 15 files changed, 29 insertions(+), 29 deletions(-) diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 0bc4b65449..1bf72a4615 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -420,8 +420,8 @@ testQemuImageCreate(const void *opaque) return -1; /* fake some sizes */ - src->capacity = 1337; - src->physical = 42; + src->capacity = UINT_MAX * 2ULL; + src->physical = UINT_MAX + 1ULL; if (qemuDomainValidateStorageSource(src, data->qemuCaps) < 0) return -1; diff --git a/tests/qemublocktestdata/imagecreate/luks-encopts.json b/tests/qemublocktestdata/imagecreate/luks-encopts.json index f065ad89a7..c5ca863f5b 100644 --- a/tests/qemublocktestdata/imagecreate/luks-encopts.json +++ b/tests/qemublocktestdata/imagecreate/luks-encopts.json @@ -2,7 +2,7 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.img", - "size": 42 + "size": 4294967296 } format: @@ -15,5 +15,5 @@ format: "ivgen-hash-alg": "sha256", "driver": "luks", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/luks-noopts.json b/tests/qemublocktestdata/imagecreate/luks-noopts.json index 1ea1948119..8a0944151d 100644 --- a/tests/qemublocktestdata/imagecreate/luks-noopts.json +++ b/tests/qemublocktestdata/imagecreate/luks-noopts.json @@ -2,7 +2,7 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.img", - "size": 42 + "size": 4294967296 } format: @@ -10,5 +10,5 @@ format: "key-secret": "0123456789ABCDEF0123456789ABCDE-encalias", "driver": "luks", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json b/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json index aee7bfd401..3e35beb088 100644 --- a/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json +++ b/tests/qemublocktestdata/imagecreate/network-gluster-qcow2.json @@ -17,12 +17,12 @@ protocol: } ] }, - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json b/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json index 56d9c0f1ed..67e2679dae 100644 --- a/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json +++ b/tests/qemublocktestdata/imagecreate/network-rbd-qcow2.json @@ -15,12 +15,12 @@ protocol: } ] }, - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json b/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json index b7272625a2..d86bef6bc8 100644 --- a/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json +++ b/tests/qemublocktestdata/imagecreate/network-sheepdog-qcow2.json @@ -9,12 +9,12 @@ protocol: }, "vdi": "asdf/i.qcow2" }, - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json index 31416ed4fc..d58054c081 100644 --- a/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json +++ b/tests/qemublocktestdata/imagecreate/network-ssh-qcow2.json @@ -8,12 +8,12 @@ protocol: "port": "1234" } }, - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json b/tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json index 63ba35dc79..5f9a800c6c 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-backing-luks.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "backing-file": "/var/lib/libvirt/images/i.img", "backing-fmt": "luks" } diff --git a/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json index b9d1d97302..c10ab98c8a 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw-nbd.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "backing-file": "nbd://example.com:1234", "backing-fmt": "raw" } diff --git a/tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json index 8176c8dadd..eb9fb413f6 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-backing-raw.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "backing-file": "/var/lib/libvirt/images/i.img", "backing-fmt": "raw" } diff --git a/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts-backing.json b/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts-backing.json index a57617dfac..641b5e04c9 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts-backing.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts-backing.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "backing-file": "/var/lib/libvirt/images/i.qcow2", "backing-fmt": "qcow2", "encrypt": { diff --git a/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json b/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json index 8796726fcb..28c85ec90b 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-luks-encopts.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "encrypt": { "key-secret": "0123456789ABCDEF0123456789ABCDE-encalias", "cipher-alg": "serpent-256", diff --git a/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json b/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json index f9caaee6bb..b5063a846d 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json +++ b/tests/qemublocktestdata/imagecreate/qcow2-luks-noopts.json @@ -2,14 +2,14 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337, + "size": 8589934590, "encrypt": { "key-secret": "0123456789ABCDEF0123456789ABCDE-encalias", "format": "luks" diff --git a/tests/qemublocktestdata/imagecreate/qcow2.json b/tests/qemublocktestdata/imagecreate/qcow2.json index 7142cf67b6..732763b763 100644 --- a/tests/qemublocktestdata/imagecreate/qcow2.json +++ b/tests/qemublocktestdata/imagecreate/qcow2.json @@ -2,12 +2,12 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.qcow2", - "size": 42 + "size": 4294967296 } format: { "driver": "qcow2", "file": "0123456789ABCDEF0123456789ABCDE", - "size": 1337 + "size": 8589934590 } diff --git a/tests/qemublocktestdata/imagecreate/raw.json b/tests/qemublocktestdata/imagecreate/raw.json index 06abb25ab9..89a6c2d237 100644 --- a/tests/qemublocktestdata/imagecreate/raw.json +++ b/tests/qemublocktestdata/imagecreate/raw.json @@ -2,7 +2,7 @@ protocol: { "driver": "file", "filename": "/var/lib/libvirt/images/i.img", - "size": 42 + "size": 4294967296 } format: -- 2.21.0

Move all bits of the formatting of the 'backing' attribute to a single condition and make it use a single extracted copy of the backing store. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_block.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index e33aad4458..54b829efed 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1404,32 +1404,34 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) virJSONValuePtr qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src) { - bool backingSupported = src->format >= VIR_STORAGE_FILE_BACKING; + virStorageSourcePtr backingStore = src->backingStore; VIR_AUTOPTR(virJSONValue) props = NULL; - if (virStorageSourceHasBacking(src) && !backingSupported) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("storage format '%s' does not support backing store"), - virStorageFileFormatTypeToString(src->format)); - return NULL; - } - if (!(props = qemuBlockStorageSourceGetBlockdevFormatProps(src))) return NULL; if (virJSONValueObjectAppendString(props, "file", src->nodestorage) < 0) return NULL; - if (src->backingStore && backingSupported) { - if (virStorageSourceHasBacking(src)) { - if (virJSONValueObjectAppendString(props, "backing", - src->backingStore->nodeformat) < 0) - return NULL; + if (backingStore) { + if (src->format >= VIR_STORAGE_FILE_BACKING) { + if (virStorageSourceIsBacking(backingStore)) { + if (virJSONValueObjectAppendString(props, "backing", + backingStore->nodeformat) < 0) + return NULL; + } else { + /* chain is terminated, indicate that no detection should happen + * in qemu */ + if (virJSONValueObjectAppendNull(props, "backing") < 0) + return NULL; + } } else { - /* chain is terminated, indicate that no detection should happen - * in qemu */ - if (virJSONValueObjectAppendNull(props, "backing") < 0) + if (virStorageSourceIsBacking(backingStore)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("storage format '%s' does not support backing store"), + virStorageFileFormatTypeToString(src->format)); return NULL; + } } } -- 2.21.0

Pass in backing store explicitly to qemuBlockStorageSourceGetBlockdevProps and fix the callers. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_block.c | 8 +++++--- src/qemu/qemu_block.h | 3 ++- tests/qemublocktest.c | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 54b829efed..fb631276c9 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1396,15 +1396,16 @@ qemuBlockStorageSourceGetBlockdevFormatProps(virStorageSourcePtr src) * qemuBlockStorageSourceGetBlockdevProps: * * @src: storage source to format + * @backingStore: a storage source to use as backing of @src * * Formats @src into a JSON object which can be used with blockdev-add or * -blockdev. The formatted object contains both the storage and format layer * in nested form including link to the backing chain layer if necessary. */ virJSONValuePtr -qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src) +qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src, + virStorageSourcePtr backingStore) { - virStorageSourcePtr backingStore = src->backingStore; VIR_AUTOPTR(virJSONValue) props = NULL; if (!(props = qemuBlockStorageSourceGetBlockdevFormatProps(src))) @@ -1484,7 +1485,8 @@ qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src, if (VIR_ALLOC(data) < 0) return NULL; - if (!(data->formatProps = qemuBlockStorageSourceGetBlockdevProps(src)) || + if (!(data->formatProps = qemuBlockStorageSourceGetBlockdevProps(src, + src->backingStore)) || !(data->storageProps = qemuBlockStorageSourceGetBackendProps(src, false, false, autoreadonly))) diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index a5e970fa1e..8c96a9b940 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -66,7 +66,8 @@ virURIPtr qemuBlockStorageSourceGetURI(virStorageSourcePtr src); virJSONValuePtr -qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src); +qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src, + virStorageSourcePtr backingStore); virJSONValuePtr qemuBlockStorageGetCopyOnReadProps(virDomainDiskDefPtr disk); diff --git a/tests/qemublocktest.c b/tests/qemublocktest.c index 1bf72a4615..bb0579056e 100644 --- a/tests/qemublocktest.c +++ b/tests/qemublocktest.c @@ -226,7 +226,7 @@ testQemuDiskXMLToProps(const void *opaque) if (qemuDomainPrepareDiskSourceData(disk, n, NULL, data->qemuCaps) < 0) goto cleanup; - if (!(formatProps = qemuBlockStorageSourceGetBlockdevProps(n)) || + if (!(formatProps = qemuBlockStorageSourceGetBlockdevProps(n, n->backingStore)) || !(storageSrcOnlyProps = qemuBlockStorageSourceGetBackendProps(n, false, true, true)) || !(storageProps = qemuBlockStorageSourceGetBackendProps(n, false, false, true))) { if (!data->fail) { -- 2.21.0

Extract the loop and supporting infrastructure to the caller as only one of the two callers actually cares about looping and rename the helper to qemuBuildStorageSourceChainAttachPrepareBlockdevOne. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 57 +++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 25 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1ca1ecd2f0..05c35919bf 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10720,33 +10720,23 @@ qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk, } -static qemuBlockStorageSourceChainDataPtr -qemuBuildStorageSourceChainAttachPrepareBlockdevInternal(virStorageSourcePtr top, - virQEMUCapsPtr qemuCaps, - bool onlyTop) +static int +qemuBuildStorageSourceChainAttachPrepareBlockdevOne(qemuBlockStorageSourceChainData *data, + virStorageSourcePtr src, + virQEMUCapsPtr qemuCaps) { VIR_AUTOPTR(qemuBlockStorageSourceAttachData) elem = NULL; - VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; - virStorageSourcePtr n; - - if (VIR_ALLOC(data) < 0) - return NULL; - - for (n = top; virStorageSourceIsBacking(n); n = n->backingStore) { - if (!(elem = qemuBlockStorageSourceAttachPrepareBlockdev(n, true))) - return NULL; - if (qemuBuildStorageSourceAttachPrepareCommon(n, elem, qemuCaps) < 0) - return NULL; + if (!(elem = qemuBlockStorageSourceAttachPrepareBlockdev(src, true))) + return -1; - if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem) < 0) - return NULL; + if (qemuBuildStorageSourceAttachPrepareCommon(src, elem, qemuCaps) < 0) + return -1; - if (onlyTop) - break; - } + if (VIR_APPEND_ELEMENT(data->srcdata, data->nsrcdata, elem) < 0) + return -1; - VIR_RETURN_PTR(data); + return 0; } @@ -10762,8 +10752,18 @@ qemuBlockStorageSourceChainDataPtr qemuBuildStorageSourceChainAttachPrepareBlockdev(virStorageSourcePtr top, virQEMUCapsPtr qemuCaps) { - return qemuBuildStorageSourceChainAttachPrepareBlockdevInternal(top, qemuCaps, - false); + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + virStorageSourcePtr n; + + if (VIR_ALLOC(data) < 0) + return NULL; + + for (n = top; virStorageSourceIsBacking(n); n = n->backingStore) { + if (qemuBuildStorageSourceChainAttachPrepareBlockdevOne(data, n, qemuCaps) < 0) + return NULL; + } + + VIR_RETURN_PTR(data); } @@ -10779,6 +10779,13 @@ qemuBlockStorageSourceChainDataPtr qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSourcePtr top, virQEMUCapsPtr qemuCaps) { - return qemuBuildStorageSourceChainAttachPrepareBlockdevInternal(top, qemuCaps, - true); + VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; + + if (VIR_ALLOC(data) < 0) + return NULL; + + if (qemuBuildStorageSourceChainAttachPrepareBlockdevOne(data, top, qemuCaps) < 0) + return NULL; + + VIR_RETURN_PTR(data); } -- 2.21.0

Pass backing store as an argument rather than extracting it locally and fix the callers. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_block.c | 4 +++- src/qemu/qemu_block.h | 1 + src/qemu/qemu_command.c | 2 +- src/qemu/qemu_migration.c | 4 +++- 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index fb631276c9..4b5dd30e17 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1464,6 +1464,7 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) /** * qemuBlockStorageSourceAttachPrepareBlockdev: * @src: storage source to prepare data from + * @backingStore: storage source to use as backing of @src * @autoreadonly: use 'auto-read-only' feature of qemu * * Creates a qemuBlockStorageSourceAttachData structure containing data to attach @@ -1478,6 +1479,7 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) */ qemuBlockStorageSourceAttachDataPtr qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src, + virStorageSourcePtr backingStore, bool autoreadonly) { VIR_AUTOPTR(qemuBlockStorageSourceAttachData) data = NULL; @@ -1486,7 +1488,7 @@ qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src, return NULL; if (!(data->formatProps = qemuBlockStorageSourceGetBlockdevProps(src, - src->backingStore)) || + backingStore)) || !(data->storageProps = qemuBlockStorageSourceGetBackendProps(src, false, false, autoreadonly))) diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 8c96a9b940..2de614b159 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -109,6 +109,7 @@ VIR_DEFINE_AUTOPTR_FUNC(qemuBlockStorageSourceAttachData, qemuBlockStorageSourceAttachDataPtr qemuBlockStorageSourceAttachPrepareBlockdev(virStorageSourcePtr src, + virStorageSourcePtr backingStore, bool autoreadonly); qemuBlockStorageSourceAttachDataPtr diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 05c35919bf..fa8bf39359 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10727,7 +10727,7 @@ qemuBuildStorageSourceChainAttachPrepareBlockdevOne(qemuBlockStorageSourceChainD { VIR_AUTOPTR(qemuBlockStorageSourceAttachData) elem = NULL; - if (!(elem = qemuBlockStorageSourceAttachPrepareBlockdev(src, true))) + if (!(elem = qemuBlockStorageSourceAttachPrepareBlockdev(src, src->backingStore, true))) return -1; if (qemuBuildStorageSourceAttachPrepareCommon(src, elem, qemuCaps) < 0) diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index bd40a8e70d..e387deb497 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -832,7 +832,9 @@ qemuMigrationSrcNBDStorageCopyBlockdev(virQEMUDriverPtr driver, /* Migration via blockdev-mirror was supported sooner than the auto-read-only * feature was added to qemu */ - if (!(data = qemuBlockStorageSourceAttachPrepareBlockdev(copysrc, false))) + if (!(data = qemuBlockStorageSourceAttachPrepareBlockdev(copysrc, + copysrc->backingStore, + false))) goto cleanup; if (qemuDomainObjEnterMonitorAsync(driver, vm, -- 2.21.0

In some cases we'll need to pass in a backing store which is not recorded as the backing store of @src. Export backingStore as variable and fix all callers to pass in the backing store. No semantic changes for now. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_command.c | 12 +++++++++--- src/qemu/qemu_command.h | 1 + src/qemu/qemu_driver.c | 1 + 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index fa8bf39359..9cd46e8ea7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10723,11 +10723,12 @@ qemuBuildStorageSourceChainAttachPrepareDrive(virDomainDiskDefPtr disk, static int qemuBuildStorageSourceChainAttachPrepareBlockdevOne(qemuBlockStorageSourceChainData *data, virStorageSourcePtr src, + virStorageSourcePtr backingStore, virQEMUCapsPtr qemuCaps) { VIR_AUTOPTR(qemuBlockStorageSourceAttachData) elem = NULL; - if (!(elem = qemuBlockStorageSourceAttachPrepareBlockdev(src, src->backingStore, true))) + if (!(elem = qemuBlockStorageSourceAttachPrepareBlockdev(src, backingStore, true))) return -1; if (qemuBuildStorageSourceAttachPrepareCommon(src, elem, qemuCaps) < 0) @@ -10759,7 +10760,9 @@ qemuBuildStorageSourceChainAttachPrepareBlockdev(virStorageSourcePtr top, return NULL; for (n = top; virStorageSourceIsBacking(n); n = n->backingStore) { - if (qemuBuildStorageSourceChainAttachPrepareBlockdevOne(data, n, qemuCaps) < 0) + if (qemuBuildStorageSourceChainAttachPrepareBlockdevOne(data, n, + n->backingStore, + qemuCaps) < 0) return NULL; } @@ -10770,6 +10773,7 @@ qemuBuildStorageSourceChainAttachPrepareBlockdev(virStorageSourcePtr top, /** * qemuBuildStorageSourceChainAttachPrepareBlockdevTop: * @top: storage source chain + * @backingStore: a storage source to use as backing of @top * @qemuCaps: qemu capabilities object * * Prepares qemuBlockStorageSourceChainDataPtr for attaching of @top image only @@ -10777,6 +10781,7 @@ qemuBuildStorageSourceChainAttachPrepareBlockdev(virStorageSourcePtr top, */ qemuBlockStorageSourceChainDataPtr qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSourcePtr top, + virStorageSourcePtr backingStore, virQEMUCapsPtr qemuCaps) { VIR_AUTOPTR(qemuBlockStorageSourceChainData) data = NULL; @@ -10784,7 +10789,8 @@ qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSourcePtr top, if (VIR_ALLOC(data) < 0) return NULL; - if (qemuBuildStorageSourceChainAttachPrepareBlockdevOne(data, top, qemuCaps) < 0) + if (qemuBuildStorageSourceChainAttachPrepareBlockdevOne(data, top, backingStore, + qemuCaps) < 0) return NULL; VIR_RETURN_PTR(data); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 6f97e7bc0c..60f9843b03 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -119,6 +119,7 @@ qemuBuildStorageSourceChainAttachPrepareBlockdev(virStorageSourcePtr top, qemuBlockStorageSourceChainDataPtr qemuBuildStorageSourceChainAttachPrepareBlockdevTop(virStorageSourcePtr top, + virStorageSourcePtr backingStore, virQEMUCapsPtr qemuCaps); char diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 78f5471b79..df677d8f4c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18547,6 +18547,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, } if (!(crdata = qemuBuildStorageSourceChainAttachPrepareBlockdevTop(mirror, + mirror->backingStore, priv->qemuCaps))) goto endjob; } -- 2.21.0

The code preparing data for creating/attaching the target image of block copy didn't use the correct reference to the existing backing chain in case when the copy should inherit it. This meant that qemu actually opened a second copy of the chain and operated on that. This would de-sync qemu from libvirt's view of node names. Luckily this is only hypothetical at this point since it happens only when -blockdev is enabled. Fix it by passing 'mirrorBacking' which has the proper data as the backing store when calling qemuBuildStorageSourceChainAttachPrepareBlockdevTop. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index df677d8f4c..1fa8fe8391 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -18544,10 +18544,12 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, * new image must refer to it in the metadata */ mirrorBacking = disk->src->backingStore; } + } else { + mirrorBacking = mirror->backingStore; } if (!(crdata = qemuBuildStorageSourceChainAttachPrepareBlockdevTop(mirror, - mirror->backingStore, + mirrorBacking, priv->qemuCaps))) goto endjob; } -- 2.21.0

When we take a snapshot we must properly remove our locking infrastructure locks. This was broken by commit 3817fa10c4ad9 which attempted to properly track the readonly state for the image as the locking code was executed after this change. Since we forced the image which was locked as read-write to read-only prior to unlocking it the write lock was not dropped. Fix it by moving the locking code prior to modifying the readonly flag. https://bugzilla.redhat.com/show_bug.cgi?id=1745618 Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1fa8fe8391..ccbec5408d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15432,6 +15432,13 @@ qemuDomainSnapshotUpdateDiskSources(virQEMUDriverPtr driver, if (qemuSecurityMoveImageMetadata(driver, vm, dd->disk->src, dd->src) < 0) VIR_WARN("Unable to move disk metadata on vm %s", vm->def->name); + /* unlock the write lock on the original image as qemu will no longer write to it */ + virDomainLockImageDetach(driver->lockManager, vm, dd->disk->src); + + /* unlock also the new image if the VM is paused to follow the locking semantics */ + if (virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING) + virDomainLockImageDetach(driver->lockManager, vm, dd->src); + /* the old disk image is now readonly */ dd->disk->src->readonly = true; @@ -15550,23 +15557,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = 0; cleanup: - if (ret < 0) { + if (ret < 0) virErrorPreserveLast(&orig_err); - } else { - /* on successful snapshot we need to remove locks from the now-old - * disks and if the VM is paused release locks on the images since qemu - * stopped using them*/ - bool paused = virDomainObjGetState(vm, NULL) != VIR_DOMAIN_RUNNING; - - for (i = 0; i < ndiskdata; i++) { - if (paused) - virDomainLockImageDetach(driver->lockManager, vm, - diskdata[i].disk->src); - - virDomainLockImageDetach(driver->lockManager, vm, - diskdata[i].disk->src->backingStore); - } - } if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0 || (vm->newDef && virDomainSaveConfig(cfg->configDir, driver->caps, -- 2.21.0

We changed to always saving the status and config XMLs to simplify code. After a few more refactors it's now possible to move it to the appropriate place and save the XMLs only on success again. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ccbec5408d..fd9dbe854d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15554,17 +15554,17 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (rc < 0) goto cleanup; + if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0 || + (vm->newDef && virDomainSaveConfig(cfg->configDir, driver->caps, + vm->newDef) < 0)) + goto cleanup; + ret = 0; cleanup: if (ret < 0) virErrorPreserveLast(&orig_err); - if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0 || - (vm->newDef && virDomainSaveConfig(cfg->configDir, driver->caps, - vm->newDef) < 0)) - ret = -1; - qemuDomainSnapshotDiskDataCleanup(diskdata, ndiskdata, driver, vm); virErrorRestore(&orig_err); -- 2.21.0

Make qemuDomainSnapshotDiskDataCleanup cleanup section friendly by moving the error preservation code inside it. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fd9dbe854d..139a8eff03 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15271,11 +15271,14 @@ qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data, virQEMUDriverPtr driver, virDomainObjPtr vm) { + virErrorPtr orig_err; size_t i; if (!data) return; + virErrorPreserveLast(&orig_err); + for (i = 0; i < ndata; i++) { /* on success of the snapshot the 'src' and 'persistsrc' properties will * be set to NULL by qemuDomainSnapshotUpdateDiskSources */ @@ -15299,6 +15302,7 @@ qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data, } VIR_FREE(data); + virErrorRestore(&orig_err); } @@ -15503,7 +15507,6 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; qemuDomainSnapshotDiskDataPtr diskdata = NULL; size_t ndiskdata = 0; - virErrorPtr orig_err = NULL; if (virDomainObjCheckActive(vm) < 0) return -1; @@ -15562,12 +15565,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = 0; cleanup: - if (ret < 0) - virErrorPreserveLast(&orig_err); - qemuDomainSnapshotDiskDataCleanup(diskdata, ndiskdata, driver, vm); - virErrorRestore(&orig_err); - return ret; } -- 2.21.0

Fix and unify the naming of external snapshot preparation functions. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 64 +++++++++++++++++++++--------------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 139a8eff03..60bc725b09 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15266,10 +15266,10 @@ typedef qemuDomainSnapshotDiskData *qemuDomainSnapshotDiskDataPtr; static void -qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data, - size_t ndata, - virQEMUDriverPtr driver, - virDomainObjPtr vm) +qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, + size_t ndata, + virQEMUDriverPtr driver, + virDomainObjPtr vm) { virErrorPtr orig_err; size_t i; @@ -15281,7 +15281,7 @@ qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data, for (i = 0; i < ndata; i++) { /* on success of the snapshot the 'src' and 'persistsrc' properties will - * be set to NULL by qemuDomainSnapshotUpdateDiskSources */ + * be set to NULL by qemuDomainSnapshotDiskUpdateSource */ if (data[i].src) { if (data[i].created && virStorageFileUnlink(data[i].src) < 0) { @@ -15307,12 +15307,12 @@ qemuDomainSnapshotDiskDataCleanup(qemuDomainSnapshotDiskDataPtr data, static int -qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainDiskDefPtr disk, - virDomainSnapshotDiskDefPtr snapdisk, - qemuDomainSnapshotDiskDataPtr dd, - bool reuse) +qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainDiskDefPtr disk, + virDomainSnapshotDiskDefPtr snapdisk, + qemuDomainSnapshotDiskDataPtr dd, + bool reuse) { char *backingStoreStr; virDomainDiskDefPtr persistdisk; @@ -15363,18 +15363,18 @@ qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr driver, /** - * qemuDomainSnapshotDiskDataCollect: + * qemuDomainSnapshotDiskPrepare: * * Collects and prepares a list of structures that hold information about disks * that are selected for the snapshot. */ static int -qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virDomainMomentObjPtr snap, - bool reuse, - qemuDomainSnapshotDiskDataPtr *rdata, - size_t *rndata) +qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, + virDomainObjPtr vm, + virDomainMomentObjPtr snap, + bool reuse, + qemuDomainSnapshotDiskDataPtr *rdata, + size_t *rndata) { size_t i; qemuDomainSnapshotDiskDataPtr data; @@ -15389,9 +15389,9 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, if (snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) continue; - if (qemuDomainSnapshotDiskDataCollectOne(driver, vm, vm->def->disks[i], - snapdef->disks + i, - data + ndata++, reuse) < 0) + if (qemuDomainSnapshotDiskPrepareOne(driver, vm, vm->def->disks[i], + snapdef->disks + i, + data + ndata++, reuse) < 0) goto cleanup; } @@ -15400,13 +15400,13 @@ qemuDomainSnapshotDiskDataCollect(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuDomainSnapshotDiskDataCleanup(data, ndata, driver, vm); + qemuDomainSnapshotDiskCleanup(data, ndata, driver, vm); return ret; } static void -qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src) +qemuDomainSnapshotDiskUpdateSourceRenumber(virStorageSourcePtr src) { virStorageSourcePtr next; unsigned int idx = 1; @@ -15417,7 +15417,7 @@ qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src) /** - * qemuDomainSnapshotUpdateDiskSources: + * qemuDomainSnapshotDiskUpdateSource: * @driver: QEMU driver * @vm: domain object * @dd: snapshot disk data object @@ -15425,9 +15425,9 @@ qemuDomainSnapshotUpdateDiskSourcesRenumber(virStorageSourcePtr src) * Updates disk definition after a successful snapshot. */ static void -qemuDomainSnapshotUpdateDiskSources(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainSnapshotDiskDataPtr dd) +qemuDomainSnapshotDiskUpdateSource(virQEMUDriverPtr driver, + virDomainObjPtr vm, + qemuDomainSnapshotDiskDataPtr dd) { /* storage driver access won'd be needed */ if (dd->initialized) @@ -15451,7 +15451,7 @@ qemuDomainSnapshotUpdateDiskSources(virQEMUDriverPtr driver, VIR_STEAL_PTR(dd->disk->src, dd->src); /* fix numbering of disks */ - qemuDomainSnapshotUpdateDiskSourcesRenumber(dd->disk->src); + qemuDomainSnapshotDiskUpdateSourceRenumber(dd->disk->src); if (dd->persistdisk) { VIR_STEAL_PTR(dd->persistsrc->backingStore, dd->persistdisk->src); @@ -15516,8 +15516,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ - if (qemuDomainSnapshotDiskDataCollect(driver, vm, snap, reuse, - &diskdata, &ndiskdata) < 0) + if (qemuDomainSnapshotDiskPrepare(driver, vm, snap, reuse, + &diskdata, &ndiskdata) < 0) goto cleanup; /* check whether there's anything to do */ @@ -15551,7 +15551,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0); if (rc == 0) - qemuDomainSnapshotUpdateDiskSources(driver, vm, dd); + qemuDomainSnapshotDiskUpdateSource(driver, vm, dd); } if (rc < 0) @@ -15565,7 +15565,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuDomainSnapshotDiskDataCleanup(diskdata, ndiskdata, driver, vm); + qemuDomainSnapshotDiskCleanup(diskdata, ndiskdata, driver, vm); return ret; } -- 2.21.0

After we always assume support for the 'transaction' command (c358adc57113b) and follow-up cleanups qemuDomainSnapshotCreateSingleDiskActive lost its value. Move the code into appropriate helpers and remove the function. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 53 ++++++++++++++++-------------------------- 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 60bc725b09..397f943a1b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15358,6 +15358,22 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, } } + /* pre-create the image file so that we can label it before handing it to qemu */ + if (!reuse && dd->src->type != VIR_STORAGE_TYPE_BLOCK) { + if (virStorageFileCreate(dd->src) < 0) { + virReportSystemError(errno, _("failed to create image file '%s'"), + NULLSTR(dd->src->path)); + return -1; + } + dd->created = true; + } + + /* set correct security, cgroup and locking options on the new image */ + if (qemuDomainStorageSourceAccessAllow(driver, vm, dd->src, false, true) < 0) + return -1; + + dd->prepared = true; + return 0; } @@ -15460,36 +15476,6 @@ qemuDomainSnapshotDiskUpdateSource(virQEMUDriverPtr driver, } -static int -qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, - virDomainObjPtr vm, - qemuDomainSnapshotDiskDataPtr dd, - virJSONValuePtr actions, - bool reuse) -{ - if (qemuBlockSnapshotAddLegacy(actions, dd->disk, dd->src, reuse) < 0) - return -1; - - /* pre-create the image file so that we can label it before handing it to qemu */ - if (!reuse && dd->src->type != VIR_STORAGE_TYPE_BLOCK) { - if (virStorageFileCreate(dd->src) < 0) { - virReportSystemError(errno, _("failed to create image file '%s'"), - NULLSTR(dd->src->path)); - return -1; - } - dd->created = true; - } - - /* set correct security, cgroup and locking options on the new image */ - if (qemuDomainStorageSourceAccessAllow(driver, vm, dd->src, false, true) < 0) - return -1; - - dd->prepared = true; - - return 0; -} - - /* The domain is expected to be locked and active. */ static int qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, @@ -15531,9 +15517,10 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, * VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL with a valid file name and * qcow2 format. */ for (i = 0; i < ndiskdata; i++) { - if (qemuDomainSnapshotCreateSingleDiskActive(driver, vm, - &diskdata[i], - actions, reuse) < 0) + if (qemuBlockSnapshotAddLegacy(actions, + diskdata[i].disk, + diskdata[i].src, + reuse) < 0) goto cleanup; } -- 2.21.0

Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 397f943a1b..39d9118953 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15356,16 +15356,16 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, else VIR_FREE(backingStoreStr); } - } - - /* pre-create the image file so that we can label it before handing it to qemu */ - if (!reuse && dd->src->type != VIR_STORAGE_TYPE_BLOCK) { - if (virStorageFileCreate(dd->src) < 0) { - virReportSystemError(errno, _("failed to create image file '%s'"), - NULLSTR(dd->src->path)); - return -1; + } else { + /* pre-create the image file so that we can label it before handing it to qemu */ + if (dd->src->type != VIR_STORAGE_TYPE_BLOCK) { + if (virStorageFileCreate(dd->src) < 0) { + virReportSystemError(errno, _("failed to create image file '%s'"), + NULLSTR(dd->src->path)); + return -1; + } + dd->created = true; } - dd->created = true; } /* set correct security, cgroup and locking options on the new image */ -- 2.21.0

Use the code for creating or attaching new storage source in the snapshot code and switch to 'blockdev-snapshot' for creating the snapshot itself. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 104 ++++++++++++++++++++++++++++++++++------- 1 file changed, 88 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 8a0f1697cb..9523da4cfc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15256,6 +15256,8 @@ struct _qemuDomainSnapshotDiskData { bool prepared; /* @src was prepared using qemuDomainStorageSourceAccessAllow */ virDomainDiskDefPtr disk; char *relPath; /* relative path component to fill into original disk */ + qemuBlockStorageSourceChainDataPtr crdata; + bool blockdevadded; virStorageSourcePtr persistsrc; virDomainDiskDefPtr persistdisk; @@ -15269,7 +15271,8 @@ static void qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, size_t ndata, virQEMUDriverPtr driver, - virDomainObjPtr vm) + virDomainObjPtr vm, + qemuDomainAsyncJob asyncJob) { virErrorPtr orig_err; size_t i; @@ -15283,6 +15286,15 @@ qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, /* on success of the snapshot the 'src' and 'persistsrc' properties will * be set to NULL by qemuDomainSnapshotDiskUpdateSource */ if (data[i].src) { + if (data[i].blockdevadded) { + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) { + + qemuBlockStorageSourceAttachRollback(qemuDomainGetMonitor(vm), + data[i].crdata->srcdata[0]); + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + } + } + if (data[i].created && virStorageFileUnlink(data[i].src) < 0) { VIR_WARN("Unable to remove just-created %s", @@ -15299,6 +15311,7 @@ qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, } virObjectUnref(data[i].persistsrc); VIR_FREE(data[i].relPath); + qemuBlockStorageSourceChainDataFree(data[i].crdata); } VIR_FREE(data); @@ -15309,15 +15322,21 @@ qemuDomainSnapshotDiskCleanup(qemuDomainSnapshotDiskDataPtr data, static int qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, virDomainObjPtr vm, + virQEMUDriverConfigPtr cfg, virDomainDiskDefPtr disk, virDomainSnapshotDiskDefPtr snapdisk, qemuDomainSnapshotDiskDataPtr dd, - bool reuse) + bool reuse, + bool blockdev, + qemuDomainAsyncJob asyncJob) { + qemuDomainObjPrivatePtr priv = vm->privateData; char *backingStoreStr; virDomainDiskDefPtr persistdisk; + VIR_AUTOUNREF(virStorageSourcePtr) terminator = NULL; bool supportsCreate; bool supportsBacking; + int rc; dd->disk = disk; @@ -15383,6 +15402,44 @@ qemuDomainSnapshotDiskPrepareOne(virQEMUDriverPtr driver, dd->prepared = true; + if (blockdev) { + /* create a terminator for the snapshot disks so that qemu does not try + * to open them at first */ + if (!(terminator = virStorageSourceNew())) + return -1; + + if (qemuDomainPrepareStorageSourceBlockdev(dd->disk, dd->src, + priv, cfg) < 0) + return -1; + + if (!(dd->crdata = qemuBuildStorageSourceChainAttachPrepareBlockdevTop(dd->src, + terminator, + priv->qemuCaps))) + return -1; + + if (reuse) { + if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) + return -1; + + rc = qemuBlockStorageSourceAttachApply(qemuDomainGetMonitor(vm), + dd->crdata->srcdata[0]); + + if (qemuDomainObjExitMonitor(driver, vm) < 0 || rc < 0) + return -1; + } else { + if (qemuBlockStorageSourceCreateDetectSize(vm, dd->src, dd->disk->src, + asyncJob) < 0) + return -1; + + if (qemuBlockStorageSourceCreate(vm, dd->src, dd->disk->src, + NULL, dd->crdata->srcdata[0], + asyncJob) < 0) + return -1; + } + + dd->blockdevadded = true; + } + return 0; } @@ -15397,7 +15454,10 @@ static int qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, virDomainObjPtr vm, virDomainMomentObjPtr snap, + virQEMUDriverConfigPtr cfg, bool reuse, + bool blockdev, + qemuDomainAsyncJob asyncJob, qemuDomainSnapshotDiskDataPtr *rdata, size_t *rndata) { @@ -15414,9 +15474,10 @@ qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, if (snapdef->disks[i].snapshot == VIR_DOMAIN_SNAPSHOT_LOCATION_NONE) continue; - if (qemuDomainSnapshotDiskPrepareOne(driver, vm, vm->def->disks[i], + if (qemuDomainSnapshotDiskPrepareOne(driver, vm, cfg, vm->def->disks[i], snapdef->disks + i, - data + ndata++, reuse) < 0) + data + ndata++, reuse, blockdev, + asyncJob) < 0) goto cleanup; } @@ -15425,7 +15486,7 @@ qemuDomainSnapshotDiskPrepare(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuDomainSnapshotDiskCleanup(data, ndata, driver, vm); + qemuDomainSnapshotDiskCleanup(data, ndata, driver, vm, asyncJob); return ret; } @@ -15446,13 +15507,15 @@ qemuDomainSnapshotDiskUpdateSourceRenumber(virStorageSourcePtr src) * @driver: QEMU driver * @vm: domain object * @dd: snapshot disk data object + * @blockdev: -blockdev is in use for the VM * * Updates disk definition after a successful snapshot. */ static void qemuDomainSnapshotDiskUpdateSource(virQEMUDriverPtr driver, virDomainObjPtr vm, - qemuDomainSnapshotDiskDataPtr dd) + qemuDomainSnapshotDiskDataPtr dd, + bool blockdev) { /* storage driver access won'd be needed */ if (dd->initialized) @@ -15476,7 +15539,8 @@ qemuDomainSnapshotDiskUpdateSource(virQEMUDriverPtr driver, VIR_STEAL_PTR(dd->disk->src, dd->src); /* fix numbering of disks */ - qemuDomainSnapshotDiskUpdateSourceRenumber(dd->disk->src); + if (!blockdev) + qemuDomainSnapshotDiskUpdateSourceRenumber(dd->disk->src); if (dd->persistdisk) { VIR_STEAL_PTR(dd->persistsrc->backingStore, dd->persistdisk->src); @@ -15502,6 +15566,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, bool reuse = (flags & VIR_DOMAIN_SNAPSHOT_CREATE_REUSE_EXT) != 0; qemuDomainSnapshotDiskDataPtr diskdata = NULL; size_t ndiskdata = 0; + bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); if (virDomainObjCheckActive(vm) < 0) return -1; @@ -15511,8 +15576,8 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, /* prepare a list of objects to use in the vm definition so that we don't * have to roll back later */ - if (qemuDomainSnapshotDiskPrepare(driver, vm, snap, reuse, - &diskdata, &ndiskdata) < 0) + if (qemuDomainSnapshotDiskPrepare(driver, vm, snap, cfg, reuse, blockdev, + asyncJob, &diskdata, &ndiskdata) < 0) goto cleanup; /* check whether there's anything to do */ @@ -15526,11 +15591,18 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, * VIR_DOMAIN_SNAPSHOT_LOCATION_EXTERNAL with a valid file name and * qcow2 format. */ for (i = 0; i < ndiskdata; i++) { - if (qemuBlockSnapshotAddLegacy(actions, - diskdata[i].disk, - diskdata[i].src, - reuse) < 0) - goto cleanup; + if (blockdev) { + if (qemuBlockSnapshotAddBlockdev(actions, + diskdata[i].disk, + diskdata[i].src)) + goto cleanup; + } else { + if (qemuBlockSnapshotAddLegacy(actions, + diskdata[i].disk, + diskdata[i].src, + reuse) < 0) + goto cleanup; + } } if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) @@ -15547,7 +15619,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, virDomainAuditDisk(vm, dd->disk->src, dd->src, "snapshot", rc >= 0); if (rc == 0) - qemuDomainSnapshotDiskUpdateSource(driver, vm, dd); + qemuDomainSnapshotDiskUpdateSource(driver, vm, dd, blockdev); } if (rc < 0) @@ -15561,7 +15633,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, ret = 0; cleanup: - qemuDomainSnapshotDiskCleanup(diskdata, ndiskdata, driver, vm); + qemuDomainSnapshotDiskCleanup(diskdata, ndiskdata, driver, vm, asyncJob); return ret; } -- 2.21.0

Remove libvit's support check for the target of an external snapshot to the blockdev code or qemu. This will potentially require a more complex cleanup but removes a level of hardcoded feature checks. Signed-off-by: Peter Krempa <pkrempa(a)redhat.com&gt; Reviewed-by: Ján Tomko <jtomko(a)redhat.com&gt; --- src/qemu/qemu_driver.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9523da4cfc..1c8d78834c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -14917,7 +14917,8 @@ qemuDomainSnapshotPrepareDiskExternalInactive(virDomainSnapshotDiskDefPtr snapdi static int qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr snapdisk, - virDomainDiskDefPtr domdisk) + virDomainDiskDefPtr domdisk, + bool blockdev) { int actualType = virStorageSourceGetActualType(snapdisk->src); @@ -14934,6 +14935,10 @@ qemuDomainSnapshotPrepareDiskExternalActive(virDomainSnapshotDiskDefPtr snapdisk break; case VIR_STORAGE_TYPE_NETWORK: + /* defer all of the checking to either qemu or libvirt's blockdev code */ + if (blockdev) + break; + switch ((virStorageNetProtocol) snapdisk->src->protocol) { case VIR_STORAGE_NET_PROTOCOL_GLUSTER: break; @@ -14981,7 +14986,8 @@ static int qemuDomainSnapshotPrepareDiskExternal(virDomainDiskDefPtr disk, virDomainSnapshotDiskDefPtr snapdisk, bool active, - bool reuse) + bool reuse, + bool blockdev) { struct stat st; int err; @@ -15004,7 +15010,7 @@ qemuDomainSnapshotPrepareDiskExternal(virDomainDiskDefPtr disk, if (qemuDomainSnapshotPrepareDiskExternalInactive(snapdisk, disk) < 0) return -1; } else { - if (qemuDomainSnapshotPrepareDiskExternalActive(snapdisk, disk) < 0) + if (qemuDomainSnapshotPrepareDiskExternalActive(snapdisk, disk, blockdev) < 0) return -1; } @@ -15105,6 +15111,8 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, virDomainSnapshotDefPtr def, unsigned int *flags) { + qemuDomainObjPrivatePtr priv = vm->privateData; + bool blockdev = virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_BLOCKDEV); int ret = -1; size_t i; bool active = virDomainObjIsActive(vm); @@ -15163,7 +15171,7 @@ qemuDomainSnapshotPrepare(virDomainObjPtr vm, } if (qemuDomainSnapshotPrepareDiskExternal(dom_disk, disk, - active, reuse) < 0) + active, reuse, blockdev) < 0) goto cleanup; external++; -- 2.21.0