[libvirt] [PATCH 0/3] misc fixes and improvements for backing chain and block devices (blockdev-add saga)
See individual patches please. Peter Krempa (3): qemu: command: Separate wrapping of disk backend props to 'file' object qemu: block: Add support for file/block/dir storage to JSON disk src generator util: storagefile: Track whether a virStorageSource was auto-detected src/qemu/qemu_block.c | 20 +++++++++----------- src/qemu/qemu_command.c | 27 ++++++++++++++++++++++++++- src/util/virstoragefile.c | 3 +++ src/util/virstoragefile.h | 2 ++ 4 files changed, 40 insertions(+), 12 deletions(-) -- 2.14.1
The file object is needed when formatting the command line, but it makes nesting of the objects less easy for use with blockdev. Separate the wrapping into the 'file' object into a helper used specifically for disk sources in the old code path. --- src/qemu/qemu_block.c | 14 +++----------- src/qemu/qemu_command.c | 27 ++++++++++++++++++++++++++- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 8d232de3e..4c0f675ca 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -572,7 +572,6 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) { int actualType = virStorageSourceGetActualType(src); virJSONValuePtr fileprops = NULL; - virJSONValuePtr ret = NULL; switch ((virStorageType) actualType) { case VIR_STORAGE_TYPE_BLOCK: @@ -587,12 +586,12 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) switch ((virStorageNetProtocol) src->protocol) { case VIR_STORAGE_NET_PROTOCOL_GLUSTER: if (!(fileprops = qemuBlockStorageSourceGetGlusterProps(src))) - goto cleanup; + return NULL; break; case VIR_STORAGE_NET_PROTOCOL_VXHS: if (!(fileprops = qemuBlockStorageSourceGetVxHSProps(src))) - goto cleanup; + return NULL; break; case VIR_STORAGE_NET_PROTOCOL_NBD: @@ -612,12 +611,5 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) break; } - if (virJSONValueObjectCreate(&ret, "a:file", fileprops, NULL) < 0) - goto cleanup; - - fileprops = NULL; - - cleanup: - virJSONValueFree(fileprops); - return ret; + return fileprops; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9c8bde49a..f6313c007 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1370,6 +1370,31 @@ qemuDiskSourceNeedsProps(virStorageSourcePtr src) } +/** + * qemuDiskSourceGetProps: + * @src: disk source struct + * + * Returns the disk source struct wrapped so that it can be used as disk source + * directly by converting it from json. + */ +static virJSONValuePtr +qemuDiskSourceGetProps(virStorageSourcePtr src) +{ + virJSONValuePtr props; + virJSONValuePtr ret; + + if (!(props = qemuBlockStorageSourceGetBackendProps(src))) + return NULL; + + if (virJSONValueObjectCreate(&ret, "a:file", props, NULL) < 0) { + virJSONValueFree(props); + return NULL; + } + + return ret; +} + + static int qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, virQEMUDriverConfigPtr cfg, @@ -1385,7 +1410,7 @@ qemuBuildDriveSourceStr(virDomainDiskDefPtr disk, int ret = -1; if (qemuDiskSourceNeedsProps(disk->src) && - !(srcprops = qemuBlockStorageSourceGetBackendProps(disk->src))) + !(srcprops = qemuDiskSourceGetProps(disk->src))) goto cleanup; if (!srcprops && -- 2.14.1
qemuBlockStorageSourceGetBackendProps now is able to format the JSON definition for regular storage too. --- src/qemu/qemu_block.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 4c0f675ca..75a8d6200 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -577,6 +577,12 @@ qemuBlockStorageSourceGetBackendProps(virStorageSourcePtr src) case VIR_STORAGE_TYPE_BLOCK: case VIR_STORAGE_TYPE_FILE: case VIR_STORAGE_TYPE_DIR: + if (virJSONValueObjectCreate(&fileprops, + "s:driver", "file", + "s:filename", src->path, NULL) < 0) + return NULL; + break; + case VIR_STORAGE_TYPE_VOLUME: case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: -- 2.14.1
When formatting an inactive or migratable XML we will need to suppress backing chain members which were detected from the disk to keep semantics straight. This means we need to record, whether a virStorageSource originates from autodetection. --- src/util/virstoragefile.c | 3 +++ src/util/virstoragefile.h | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index dd4494940..df6a547b0 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2042,6 +2042,7 @@ virStorageSourceCopy(const virStorageSource *src, ret->haveTLS = src->haveTLS; ret->tlsFromConfig = src->tlsFromConfig; ret->tlsVerify = src->tlsVerify; + ret->detected = src->detected; /* storage driver metadata are not copied */ ret->drv = NULL; @@ -3418,6 +3419,8 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent) goto error; } + ret->detected = true; + return ret; error: diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 74dee10f2..5181d6cb9 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -295,6 +295,8 @@ struct _virStorageSource { char *tlsAlias; char *tlsCertdir; bool tlsVerify; + + bool detected; /* true if this entry was not provided by the user */ }; -- 2.14.1
On 10/12/2017 03:13 PM, Peter Krempa wrote:
When formatting an inactive or migratable XML we will need to suppress backing chain members which were detected from the disk to keep semantics straight. This means we need to record, whether a virStorageSource originates from autodetection. --- src/util/virstoragefile.c | 3 +++ src/util/virstoragefile.h | 2 ++ 2 files changed, 5 insertions(+)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index dd4494940..df6a547b0 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -2042,6 +2042,7 @@ virStorageSourceCopy(const virStorageSource *src, ret->haveTLS = src->haveTLS; ret->tlsFromConfig = src->tlsFromConfig; ret->tlsVerify = src->tlsVerify; + ret->detected = src->detected;
/* storage driver metadata are not copied */ ret->drv = NULL; @@ -3418,6 +3419,8 @@ virStorageSourceNewFromBacking(virStorageSourcePtr parent) goto error; }
+ ret->detected = true; +
Dang it... Looked at the wrong tab when checking build results, compiler returns: util/virstoragefile.c: In function 'virStorageSourceNewFromBacking': util/virstoragefile.c:3440:19: error: potential null pointer dereference [-Werror=null-dereference] ret->detected = true; here for me. That needs to be fixed... John
return ret;
error: diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 74dee10f2..5181d6cb9 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -295,6 +295,8 @@ struct _virStorageSource { char *tlsAlias; char *tlsCertdir; bool tlsVerify; + + bool detected; /* true if this entry was not provided by the user */ };
On 10/12/2017 03:13 PM, Peter Krempa wrote:
See individual patches please.
Peter Krempa (3): qemu: command: Separate wrapping of disk backend props to 'file' object qemu: block: Add support for file/block/dir storage to JSON disk src generator util: storagefile: Track whether a virStorageSource was auto-detected
src/qemu/qemu_block.c | 20 +++++++++----------- src/qemu/qemu_command.c | 27 ++++++++++++++++++++++++++- src/util/virstoragefile.c | 3 +++ src/util/virstoragefile.h | 2 ++ 4 files changed, 40 insertions(+), 12 deletions(-)
Reviewed-by: John Ferlan <jferlan@redhat.com> (series) John
participants (2)
-
John Ferlan -
Peter Krempa