[libvirt] [PATCH 0/9] qemu: Refactor and simplify handling of disk hotplug and commandline
This refactor unifies setup of data for disk hotplug and command line generation. This applies on top of 'qemu: Handle managed persisten reservations separately' as that applies on top of the staging branch for my ACKed refactors which moved now, you can fetch everything at: git fetch git://pipo.sk/pipo/libvirt.git disk-hotplug-refactor Peter Krempa (9): qemu: hotplug: Remove qemuDomainDelDiskSrcTLSObject qemu: alias: Rename qemuAliasFromDisk to qemuAliasDiskDriveFromDisk qemu: Reuse qemuBlockStorageSourceAttachApply in disk hotplug qemu: hotplug: Extract hotplug of PR into qemuBlockStorageSourceAttachApply qemu: hotplug: Extract hotplug of secrets into qemuBlockStorageSourceAttachApply qemu: hotplug: Extract hotplug of TLS into qemuBlockStorageSourceAttachApply qemu: command: Rename qemuBuildDiskDriveCommandLine qemu: command: Extract setup of one disk's command line qemu: command: Refactor disk commandline formatting src/qemu/qemu_alias.c | 4 +- src/qemu/qemu_alias.h | 2 +- src/qemu/qemu_block.c | 58 +++++++++++- src/qemu/qemu_block.h | 16 ++++ src/qemu/qemu_command.c | 232 +++++++++++++++++++++++++++++----------------- src/qemu/qemu_command.h | 13 ++- src/qemu/qemu_driver.c | 18 ++-- src/qemu/qemu_hotplug.c | 121 +++--------------------- src/qemu/qemu_migration.c | 6 +- src/qemu/qemu_process.c | 2 +- 10 files changed, 257 insertions(+), 215 deletions(-) -- 2.16.2
Replace access via wrapper by direct call to monitor API. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6564615597..d8ca894519 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -182,15 +182,6 @@ qemuDomainAddDiskSrcTLSObject(virQEMUDriverPtr driver, } -static void -qemuDomainDelDiskSrcTLSObject(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virStorageSourcePtr src) -{ - qemuDomainDelTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE, NULL, src->tlsAlias); -} - - static int qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -529,6 +520,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, ignore_value(qemuMonitorDelObject(priv->mon, unmanagedPrmgrAlias)); if (managedPrmgrAlias) ignore_value(qemuMonitorDelObject(priv->mon, managedPrmgrAlias)); + if (disk->src->tlsAlias) + ignore_value(qemuMonitorDelObject(priv->mon, disk->src->tlsAlias)); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -2; virErrorRestore(&orig_err); @@ -536,7 +529,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, virDomainAuditDisk(vm, NULL, disk->src, "attach", false); error: - qemuDomainDelDiskSrcTLSObject(driver, vm, disk->src); ignore_value(qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, true)); if (priv->prDaemonRunning && !virDomainDefHasManagedPR(vm->def)) -- 2.16.2
On Fri, Jun 01, 2018 at 05:51:04PM +0200, Peter Krempa wrote:
Replace access via wrapper by direct call to monitor API.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_hotplug.c | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Emphasize that it's for the 'drive' part of the disk. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_alias.c | 4 ++-- src/qemu/qemu_alias.h | 2 +- src/qemu/qemu_block.c | 2 +- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_driver.c | 18 +++++++++--------- src/qemu/qemu_hotplug.c | 6 +++--- src/qemu/qemu_migration.c | 6 +++--- src/qemu/qemu_process.c | 2 +- 8 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index 89dec91ed1..989cbb4ba9 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -650,7 +650,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) } -/* qemuAliasFromDisk +/* qemuAliasDiskDriveFromDisk * @disk: Pointer to a disk definition * * Generate and return an alias for the device disk '-drive' @@ -658,7 +658,7 @@ qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps) * Returns NULL with error or a string containing the alias */ char * -qemuAliasFromDisk(const virDomainDiskDef *disk) +qemuAliasDiskDriveFromDisk(const virDomainDiskDef *disk) { char *ret; diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h index 51f64624d7..0e2370b737 100644 --- a/src/qemu/qemu_alias.h +++ b/src/qemu/qemu_alias.h @@ -75,7 +75,7 @@ int qemuAssignDeviceAliases(virDomainDefPtr def, virQEMUCapsPtr qemuCaps); int qemuDomainDeviceAliasIndex(const virDomainDeviceInfo *info, const char *prefix); -char *qemuAliasFromDisk(const virDomainDiskDef *disk); +char *qemuAliasDiskDriveFromDisk(const virDomainDiskDef *disk); const char *qemuAliasDiskDriveSkipPrefix(const char *dev_name); diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index d1c2d756c2..85176925c9 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -297,7 +297,7 @@ qemuBlockDiskDetectNodes(virDomainDiskDefPtr disk, if (src->nodeformat || src->nodestorage) return 0; - if (!(alias = qemuAliasFromDisk(disk))) + if (!(alias = qemuAliasDiskDriveFromDisk(disk))) goto cleanup; if (!(entry = virHashLookup(disktable, alias))) { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 1e30c79d6f..484ac261b0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1652,7 +1652,7 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk, goto error; if (qemuDiskBusNeedsDeviceArg(disk->bus)) { - char *drivealias = qemuAliasFromDisk(disk); + char *drivealias = qemuAliasDiskDriveFromDisk(disk); if (!drivealias) goto error; @@ -2087,7 +2087,7 @@ qemuBuildDriveDevStr(const virDomainDef *def, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DISK_SHARE_RW)) virBufferAddLit(&opt, ",share-rw=on"); - if (!(drivealias = qemuAliasFromDisk(disk))) + if (!(drivealias = qemuAliasDiskDriveFromDisk(disk))) goto error; virBufferAsprintf(&opt, ",drive=%s,id=%s", drivealias, disk->info.alias); VIR_FREE(drivealias); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 05a09eb706..b7c3348550 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10960,7 +10960,7 @@ qemuDomainBlockResize(virDomainPtr dom, disk->src->format == VIR_STORAGE_FILE_QED) size = VIR_ROUND_UP(size, 512); - if (!(device = qemuAliasFromDisk(disk))) + if (!(device = qemuAliasDiskDriveFromDisk(disk))) goto endjob; qemuDomainObjEnterMonitor(driver, vm); @@ -14928,7 +14928,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, const char *formatStr = NULL; int ret = -1, rc; - if (!(device = qemuAliasFromDisk(dd->disk))) + if (!(device = qemuAliasDiskDriveFromDisk(dd->disk))) goto cleanup; if (qemuGetDriveSourceString(dd->src, NULL, &source) < 0) @@ -16976,7 +16976,7 @@ qemuDomainBlockPullCommon(virQEMUDriverPtr driver, if (!(disk = qemuDomainDiskByName(vm->def, path))) goto endjob; - if (!(device = qemuAliasFromDisk(disk))) + if (!(device = qemuAliasDiskDriveFromDisk(disk))) goto endjob; if (qemuDomainDiskBlockJobIsActive(disk)) @@ -17089,7 +17089,7 @@ qemuDomainBlockJobAbort(virDomainPtr dom, if (!(disk = qemuDomainDiskByName(vm->def, path))) goto endjob; - if (!(device = qemuAliasFromDisk(disk))) + if (!(device = qemuAliasDiskDriveFromDisk(disk))) goto endjob; if (disk->mirrorState != VIR_DOMAIN_DISK_MIRROR_STATE_NONE && @@ -17320,7 +17320,7 @@ qemuDomainBlockJobSetSpeed(virDomainPtr dom, if (!(disk = qemuDomainDiskByName(vm->def, path))) goto endjob; - if (!(device = qemuAliasFromDisk(disk))) + if (!(device = qemuAliasDiskDriveFromDisk(disk))) goto endjob; qemuDomainObjEnterMonitor(driver, vm); @@ -17452,7 +17452,7 @@ qemuDomainBlockCopyCommon(virDomainObjPtr vm, if (!(disk = qemuDomainDiskByName(vm->def, path))) goto endjob; - if (!(device = qemuAliasFromDisk(disk))) + if (!(device = qemuAliasDiskDriveFromDisk(disk))) goto endjob; if (qemuDomainDiskBlockJobIsActive(disk)) @@ -17832,7 +17832,7 @@ qemuDomainBlockCommit(virDomainPtr dom, if (!(disk = qemuDomainDiskByName(vm->def, path))) goto endjob; - if (!(device = qemuAliasFromDisk(disk))) + if (!(device = qemuAliasDiskDriveFromDisk(disk))) goto endjob; if (!disk->src->path) { @@ -18463,7 +18463,7 @@ qemuDomainSetBlockIoTune(virDomainPtr dom, if (!(disk = qemuDomainDiskByName(def, path))) goto endjob; - if (!(device = qemuAliasFromDisk(disk))) + if (!(device = qemuAliasDiskDriveFromDisk(disk))) goto endjob; if (qemuDomainSetBlockIoTuneDefaults(&info, &disk->blkdeviotune, @@ -18636,7 +18636,7 @@ qemuDomainGetBlockIoTune(virDomainPtr dom, if (!(disk = qemuDomainDiskByName(def, path))) goto endjob; - if (!(device = qemuAliasFromDisk(disk))) + if (!(device = qemuAliasDiskDriveFromDisk(disk))) goto endjob; qemuDomainObjEnterMonitor(driver, vm); ret = qemuMonitorGetBlockIoThrottle(priv->mon, device, &reply); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d8ca894519..62fb652093 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -270,7 +270,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver, if (qemuHotplugPrepareDiskAccess(driver, vm, disk, newsrc, false) < 0) goto cleanup; - if (!(driveAlias = qemuAliasFromDisk(disk))) + if (!(driveAlias = qemuAliasDiskDriveFromDisk(disk))) goto error; qemuDomainObjEnterMonitor(driver, vm); @@ -446,7 +446,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) goto error; - if (!(drivealias = qemuAliasFromDisk(disk))) + if (!(drivealias = qemuAliasDiskDriveFromDisk(disk))) goto error; if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) @@ -3776,7 +3776,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, /* build the actual drive id string as the disk->info.alias doesn't * contain the QEMU_DRIVE_HOST_PREFIX that is passed to qemu */ - if (!(drivestr = qemuAliasFromDisk(disk))) + if (!(drivestr = qemuAliasDiskDriveFromDisk(disk))) return -1; if (diskPriv) { diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 5cf9be56b4..13d131f6b6 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -400,7 +400,7 @@ qemuMigrationDstStartNBDServer(virQEMUDriverPtr driver, } VIR_FREE(diskAlias); - if (!(diskAlias = qemuAliasFromDisk(disk))) + if (!(diskAlias = qemuAliasDiskDriveFromDisk(disk))) goto cleanup; if (qemuDomainObjEnterMonitorAsync(driver, vm, @@ -651,7 +651,7 @@ qemuMigrationSrcNBDCopyCancelOne(virQEMUDriverPtr driver, goto cleanup; } - if (!(diskAlias = qemuAliasFromDisk(disk))) + if (!(diskAlias = qemuAliasDiskDriveFromDisk(disk))) return -1; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) @@ -951,7 +951,7 @@ qemuMigrationSrcNBDStorageCopy(virQEMUDriverPtr driver, if (!qemuMigrationAnyCopyDisk(disk, nmigrate_disks, migrate_disks)) continue; - if (!(diskAlias = qemuAliasFromDisk(disk))) + if (!(diskAlias = qemuAliasDiskDriveFromDisk(disk))) goto cleanup; qemuBlockJobSyncBegin(disk); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 30cc5904e0..87fc6ffc67 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2774,7 +2774,7 @@ qemuProcessInitPasswords(virQEMUDriverPtr driver, goto cleanup; VIR_FREE(alias); - if (!(alias = qemuAliasFromDisk(vm->def->disks[i]))) + if (!(alias = qemuAliasDiskDriveFromDisk(vm->def->disks[i]))) goto cleanup; if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0) goto cleanup; -- 2.16.2
On Fri, Jun 01, 2018 at 05:51:05PM +0200, Peter Krempa wrote:
Emphasize that it's for the 'drive' part of the disk.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_alias.c | 4 ++-- src/qemu/qemu_alias.h | 2 +- src/qemu/qemu_block.c | 2 +- src/qemu/qemu_command.c | 4 ++-- src/qemu/qemu_driver.c | 18 +++++++++--------- src/qemu/qemu_hotplug.c | 6 +++--- src/qemu/qemu_migration.c | 6 +++--- src/qemu/qemu_process.c | 2 +- 8 files changed, 22 insertions(+), 22 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Create a new "Prepare" function and move the drive add code into the new helpers. This will eventually allow to simplify and unify the attaching code for use with blockdev at the same time as providing compatibility with older qemus. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 18 ++++++++++++++++++ src/qemu/qemu_block.h | 4 ++++ src/qemu/qemu_command.c | 29 ++++++++++++++++++++++++++++- src/qemu/qemu_command.h | 8 ++++---- src/qemu/qemu_hotplug.c | 26 +++++++++----------------- 5 files changed, 63 insertions(+), 22 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 85176925c9..73aab9d73a 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -24,9 +24,12 @@ #include "viralloc.h" #include "virstring.h" +#include "virlog.h" #define VIR_FROM_THIS VIR_FROM_QEMU +VIR_LOG_INIT("qemu.qemu_block"); + /* qemu declares the buffer for node names as a 32 byte array */ static const size_t qemuBlockNodeNameBufSize = 32; @@ -1482,6 +1485,8 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) virJSONValueFree(data->storageProps); virJSONValueFree(data->formatProps); + VIR_FREE(data->driveCmd); + VIR_FREE(data->driveAlias); VIR_FREE(data); } @@ -1563,6 +1568,13 @@ qemuBlockStorageSourceAttachApply(qemuMonitorPtr mon, data->formatAttached = true; } + if (data->driveCmd) { + if (qemuMonitorAddDrive(mon, data->driveCmd) < 0) + return -1; + + data->driveAdded = true; + } + return 0; } @@ -1591,6 +1603,12 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, if (data->storageAttached) ignore_value(qemuMonitorBlockdevDel(mon, data->storageNodeName)); + if (data->driveAdded) { + if (qemuMonitorDriveDel(mon, data->driveAlias) < 0) + VIR_WARN("Unable to remove drive %s (%s) after failed " + "qemuMonitorAddDevice", data->driveAlias, data->driveCmd); + } + virErrorRestore(&orig_err); } diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index bbaf9ec0f1..5c7791ee72 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -78,6 +78,10 @@ struct qemuBlockStorageSourceAttachData { virJSONValuePtr formatProps; const char *formatNodeName; bool formatAttached; + + char *driveCmd; + char *driveAlias; + bool driveAdded; }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 484ac261b0..84bb857507 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1639,7 +1639,7 @@ qemuBuildDiskFrontendAttributes(virDomainDiskDefPtr disk, } -char * +static char * qemuBuildDriveStr(virDomainDiskDefPtr disk, bool bootable, virQEMUCapsPtr qemuCaps) @@ -10436,3 +10436,30 @@ qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu) virJSONValueFree(ret); return NULL; } + + +/** + * qemuBuildStorageSourceAttachPrepareDrive: + * @disk: disk object to prepare + * @qemuCaps: qemu capabilities object + * + * Prepare qemuBlockStorageSourceAttachDataPtr for use with the old approach + * using -drive/drive_add. See qemuBlockStorageSourceAttachPrepareBlockdev. + */ +qemuBlockStorageSourceAttachDataPtr +qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps) +{ + qemuBlockStorageSourceAttachDataPtr data = NULL; + + if (VIR_ALLOC(data) < 0) + return NULL; + + if (!(data->driveCmd = qemuBuildDriveStr(disk, false, qemuCaps)) || + !(data->driveAlias = qemuAliasDiskDriveFromDisk(disk))) { + qemuBlockStorageSourceAttachDataFree(data); + return NULL; + } + + return data; +} diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index b7580b4796..04f6245bc7 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -28,6 +28,7 @@ # include "domain_conf.h" # include "vircommand.h" # include "capabilities.h" +# include "qemu_block.h" # include "qemu_conf.h" # include "qemu_domain.h" # include "qemu_domain_address.h" @@ -102,10 +103,9 @@ char *qemuBuildNicDevStr(virDomainDefPtr def, char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk); -/* Both legacy & current support */ -char *qemuBuildDriveStr(virDomainDiskDefPtr disk, - bool bootable, - virQEMUCapsPtr qemuCaps); +qemuBlockStorageSourceAttachDataPtr +qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps); /* Current, best practice */ char *qemuBuildDriveDevStr(const virDomainDef *def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 62fb652093..ece9a82562 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -35,6 +35,7 @@ #include "qemu_interface.h" #include "qemu_process.h" #include "qemu_security.h" +#include "qemu_block.h" #include "domain_audit.h" #include "netdev_bandwidth_conf.h" #include "domain_nwfilter.h" @@ -390,15 +391,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, { int ret = -1; qemuDomainObjPrivatePtr priv = vm->privateData; + qemuBlockStorageSourceAttachDataPtr data = NULL; virErrorPtr orig_err; char *devstr = NULL; - char *drivestr = NULL; - char *drivealias = NULL; char *unmanagedPrmgrAlias = NULL; char *managedPrmgrAlias = NULL; char *encobjAlias = NULL; char *secobjAlias = NULL; - bool driveAdded = false; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virJSONValuePtr secobjProps = NULL; virJSONValuePtr encobjProps = NULL; @@ -439,14 +438,11 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, !(unmanagedPrmgrProps = qemuBuildPRManagerInfoProps(disk->src))) goto error; - if (disk->src->haveTLS == VIR_TRISTATE_BOOL_YES && - qemuDomainAddDiskSrcTLSObject(driver, vm, disk->src) < 0) - goto error; - - if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps))) + if (!(data = qemuBuildStorageSourceAttachPrepareDrive(disk, priv->qemuCaps))) goto error; - if (!(drivealias = qemuAliasDiskDriveFromDisk(disk))) + if (disk->src->haveTLS == VIR_TRISTATE_BOOL_YES && + qemuDomainAddDiskSrcTLSObject(driver, vm, disk->src) < 0) goto error; if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) @@ -473,9 +469,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuMonitorAddObject(priv->mon, &unmanagedPrmgrProps, &unmanagedPrmgrAlias) < 0) goto exit_monitor; - if (qemuMonitorAddDrive(priv->mon, drivestr) < 0) + if (qemuBlockStorageSourceAttachApply(priv->mon, data) < 0) goto exit_monitor; - driveAdded = true; if (qemuMonitorAddDevice(priv->mon, devstr) < 0) goto exit_monitor; @@ -491,6 +486,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, ret = 0; cleanup: + qemuBlockStorageSourceAttachDataFree(data); virJSONValueFree(managedPrmgrProps); virJSONValueFree(unmanagedPrmgrProps); virJSONValueFree(encobjProps); @@ -500,18 +496,14 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, VIR_FREE(unmanagedPrmgrAlias); VIR_FREE(secobjAlias); VIR_FREE(encobjAlias); - VIR_FREE(drivealias); - VIR_FREE(drivestr); VIR_FREE(devstr); virObjectUnref(cfg); return ret; exit_monitor: + qemuBlockStorageSourceAttachRollback(priv->mon, data); + virErrorPreserveLast(&orig_err); - if (driveAdded && qemuMonitorDriveDel(priv->mon, drivealias) < 0) { - VIR_WARN("Unable to remove drive %s (%s) after failed " - "qemuMonitorAddDevice", drivealias, drivestr); - } if (secobjAlias) ignore_value(qemuMonitorDelObject(priv->mon, secobjAlias)); if (encobjAlias) -- 2.16.2
On Fri, Jun 01, 2018 at 05:51:06PM +0200, Peter Krempa wrote:
Create a new "Prepare" function and move the drive add code into the new helpers. This will eventually allow to simplify and unify the attaching code for use with blockdev at the same time as providing compatibility with older qemus.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 18 ++++++++++++++++++ src/qemu/qemu_block.h | 4 ++++ src/qemu/qemu_command.c | 29 ++++++++++++++++++++++++++++- src/qemu/qemu_command.h | 8 ++++---- src/qemu/qemu_hotplug.c | 26 +++++++++----------------- 5 files changed, 63 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 85176925c9..73aab9d73a 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -24,9 +24,12 @@
#include "viralloc.h" #include "virstring.h" +#include "virlog.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
+VIR_LOG_INIT("qemu.qemu_block"); + /* qemu declares the buffer for node names as a 32 byte array */ static const size_t qemuBlockNodeNameBufSize = 32;
@@ -1482,6 +1485,8 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data)
virJSONValueFree(data->storageProps); virJSONValueFree(data->formatProps); + VIR_FREE(data->driveCmd); + VIR_FREE(data->driveAlias); VIR_FREE(data); }
@@ -1563,6 +1568,13 @@ qemuBlockStorageSourceAttachApply(qemuMonitorPtr mon, data->formatAttached = true; }
+ if (data->driveCmd) { + if (qemuMonitorAddDrive(mon, data->driveCmd) < 0) + return -1; + + data->driveAdded = true; + } + return 0; }
@@ -1591,6 +1603,12 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, if (data->storageAttached) ignore_value(qemuMonitorBlockdevDel(mon, data->storageNodeName));
+ if (data->driveAdded) { + if (qemuMonitorDriveDel(mon, data->driveAlias) < 0) + VIR_WARN("Unable to remove drive %s (%s) after failed " + "qemuMonitorAddDevice", data->driveAlias, data->driveCmd); + } + virErrorRestore(&orig_err);
Even though this call is unrelated to the other two, shouldn't rollback be in reverse order? Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
On Sat, Jun 02, 2018 at 17:39:51 +0200, Ján Tomko wrote:
On Fri, Jun 01, 2018 at 05:51:06PM +0200, Peter Krempa wrote:
Create a new "Prepare" function and move the drive add code into the new helpers. This will eventually allow to simplify and unify the attaching code for use with blockdev at the same time as providing compatibility with older qemus.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 18 ++++++++++++++++++ src/qemu/qemu_block.h | 4 ++++ src/qemu/qemu_command.c | 29 ++++++++++++++++++++++++++++- src/qemu/qemu_command.h | 8 ++++---- src/qemu/qemu_hotplug.c | 26 +++++++++----------------- 5 files changed, 63 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 85176925c9..73aab9d73a 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c
[...]
@@ -1591,6 +1603,12 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, if (data->storageAttached) ignore_value(qemuMonitorBlockdevDel(mon, data->storageNodeName));
+ if (data->driveAdded) { + if (qemuMonitorDriveDel(mon, data->driveAlias) < 0) + VIR_WARN("Unable to remove drive %s (%s) after failed " + "qemuMonitorAddDevice", data->driveAlias, data->driveCmd); + } + virErrorRestore(&orig_err);
Even though this call is unrelated to the other two, shouldn't rollback be in reverse order?
I'll put it so that it's strictly in reverse order, but I made it impossible to create the data structure in a way that would allow 'drive_add' and 'blockdev_add' be used at the same time.
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Introduce a new setup function for all the related configuration and move the setup and attachment of the PR code. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 8 ++++++++ src/qemu/qemu_block.h | 3 +++ src/qemu/qemu_command.c | 21 +++++++++++++++++++++ src/qemu/qemu_command.h | 3 +++ src/qemu/qemu_hotplug.c | 16 ++-------------- 5 files changed, 37 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 73aab9d73a..2bc8120f5f 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1485,6 +1485,7 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) virJSONValueFree(data->storageProps); virJSONValueFree(data->formatProps); + virJSONValueFree(data->prmgrProps); VIR_FREE(data->driveCmd); VIR_FREE(data->driveAlias); VIR_FREE(data); @@ -1548,6 +1549,10 @@ qemuBlockStorageSourceAttachApply(qemuMonitorPtr mon, { int rv; + if (data->prmgrProps && + qemuMonitorAddObject(mon, &data->prmgrProps, &data->prmgrAlias) < 0) + return -1; + if (data->storageProps) { rv = qemuMonitorBlockdevAdd(mon, data->storageProps); data->storageProps = NULL; @@ -1609,6 +1614,9 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, "qemuMonitorAddDevice", data->driveAlias, data->driveCmd); } + if (data->prmgrAlias) + ignore_value(qemuMonitorDelObject(mon, data->prmgrAlias)); + virErrorRestore(&orig_err); } diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 5c7791ee72..e5064574a9 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -71,6 +71,9 @@ qemuBlockStorageSourceGetBlockdevProps(virStorageSourcePtr src); typedef struct qemuBlockStorageSourceAttachData qemuBlockStorageSourceAttachData; typedef qemuBlockStorageSourceAttachData *qemuBlockStorageSourceAttachDataPtr; struct qemuBlockStorageSourceAttachData { + virJSONValuePtr prmgrProps; + char *prmgrAlias; + virJSONValuePtr storageProps; const char *storageNodeName; bool storageAttached; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 84bb857507..af97069770 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10463,3 +10463,24 @@ qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, return data; } + + +/** + * qemuBuildStorageSourceAttachPrepareCommon: + * @src: storage source + * @data: already initialized data for disk source addition + * + * Prepare data for configuration associated with the disk source such as + * secrets/TLS/pr objects etc ... + */ +int +qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, + qemuBlockStorageSourceAttachDataPtr data) +{ + if (src->pr && + !virStoragePRDefIsManaged(src->pr) && + !(data->prmgrProps = qemuBuildPRManagerInfoProps(src))) + return -1; + + return 0; +} diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 04f6245bc7..711fce9648 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -106,6 +106,9 @@ char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk); qemuBlockStorageSourceAttachDataPtr qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps); +int +qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, + qemuBlockStorageSourceAttachDataPtr data); /* Current, best practice */ char *qemuBuildDriveDevStr(const virDomainDef *def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index ece9a82562..7ea5a531f3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -394,14 +394,12 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuBlockStorageSourceAttachDataPtr data = NULL; virErrorPtr orig_err; char *devstr = NULL; - char *unmanagedPrmgrAlias = NULL; char *managedPrmgrAlias = NULL; char *encobjAlias = NULL; char *secobjAlias = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virJSONValuePtr secobjProps = NULL; virJSONValuePtr encobjProps = NULL; - virJSONValuePtr unmanagedPrmgrProps = NULL; virJSONValuePtr managedPrmgrProps = NULL; qemuDomainStorageSourcePrivatePtr srcPriv; qemuDomainSecretInfoPtr secinfo = NULL; @@ -433,12 +431,10 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (qemuDomainDiskAttachManagedPR(vm, disk, &managedPrmgrProps) < 0) goto error; - if (disk->src->pr && - !virStoragePRDefIsManaged(disk->src->pr) && - !(unmanagedPrmgrProps = qemuBuildPRManagerInfoProps(disk->src))) + if (!(data = qemuBuildStorageSourceAttachPrepareDrive(disk, priv->qemuCaps))) goto error; - if (!(data = qemuBuildStorageSourceAttachPrepareDrive(disk, priv->qemuCaps))) + if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, data) < 0) goto error; if (disk->src->haveTLS == VIR_TRISTATE_BOOL_YES && @@ -465,10 +461,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuMonitorAddObject(priv->mon, &managedPrmgrProps, &managedPrmgrAlias) < 0) goto exit_monitor; - if (unmanagedPrmgrProps && - qemuMonitorAddObject(priv->mon, &unmanagedPrmgrProps, &unmanagedPrmgrAlias) < 0) - goto exit_monitor; - if (qemuBlockStorageSourceAttachApply(priv->mon, data) < 0) goto exit_monitor; @@ -488,12 +480,10 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, cleanup: qemuBlockStorageSourceAttachDataFree(data); virJSONValueFree(managedPrmgrProps); - virJSONValueFree(unmanagedPrmgrProps); virJSONValueFree(encobjProps); virJSONValueFree(secobjProps); qemuDomainSecretDiskDestroy(disk); VIR_FREE(managedPrmgrAlias); - VIR_FREE(unmanagedPrmgrAlias); VIR_FREE(secobjAlias); VIR_FREE(encobjAlias); VIR_FREE(devstr); @@ -508,8 +498,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, ignore_value(qemuMonitorDelObject(priv->mon, secobjAlias)); if (encobjAlias) ignore_value(qemuMonitorDelObject(priv->mon, encobjAlias)); - if (unmanagedPrmgrAlias) - ignore_value(qemuMonitorDelObject(priv->mon, unmanagedPrmgrAlias)); if (managedPrmgrAlias) ignore_value(qemuMonitorDelObject(priv->mon, managedPrmgrAlias)); if (disk->src->tlsAlias) -- 2.16.2
On Fri, Jun 01, 2018 at 05:51:07PM +0200, Peter Krempa wrote:
Introduce a new setup function for all the related configuration and move the setup and attachment of the PR code.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 8 ++++++++ src/qemu/qemu_block.h | 3 +++ src/qemu/qemu_command.c | 21 +++++++++++++++++++++ src/qemu/qemu_command.h | 3 +++ src/qemu/qemu_hotplug.c | 16 ++-------------- 5 files changed, 37 insertions(+), 14 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 21 +++++++++++++++++++++ src/qemu/qemu_block.h | 6 ++++++ src/qemu/qemu_command.c | 13 +++++++++++++ src/qemu/qemu_hotplug.c | 37 ------------------------------------- 4 files changed, 40 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 2bc8120f5f..b6b1316ea5 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1486,6 +1486,10 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) virJSONValueFree(data->storageProps); virJSONValueFree(data->formatProps); virJSONValueFree(data->prmgrProps); + virJSONValueFree(data->authsecretProps); + virJSONValueFree(data->encryptsecretProps); + VIR_FREE(data->authsecretAlias); + VIR_FREE(data->encryptsecretAlias); VIR_FREE(data->driveCmd); VIR_FREE(data->driveAlias); VIR_FREE(data); @@ -1553,6 +1557,16 @@ qemuBlockStorageSourceAttachApply(qemuMonitorPtr mon, qemuMonitorAddObject(mon, &data->prmgrProps, &data->prmgrAlias) < 0) return -1; + if (data->authsecretProps && + qemuMonitorAddObject(mon, &data->authsecretProps, + &data->authsecretAlias) < 0) + return -1; + + if (data->encryptsecretProps && + qemuMonitorAddObject(mon, &data->encryptsecretProps, + &data->encryptsecretAlias) < 0) + return -1; + if (data->storageProps) { rv = qemuMonitorBlockdevAdd(mon, data->storageProps); data->storageProps = NULL; @@ -1617,6 +1631,13 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, if (data->prmgrAlias) ignore_value(qemuMonitorDelObject(mon, data->prmgrAlias)); + if (data->authsecretAlias) + ignore_value(qemuMonitorDelObject(mon, data->authsecretAlias)); + + if (data->encryptsecretAlias) + ignore_value(qemuMonitorDelObject(mon, data->encryptsecretAlias)); + + virErrorRestore(&orig_err); } diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index e5064574a9..4ffb42dfd6 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -85,6 +85,12 @@ struct qemuBlockStorageSourceAttachData { char *driveCmd; char *driveAlias; bool driveAdded; + + virJSONValuePtr authsecretProps; + char *authsecretAlias; + + virJSONValuePtr encryptsecretProps; + char *encryptsecretAlias; }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index af97069770..e11ae8b874 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10477,10 +10477,23 @@ int qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, qemuBlockStorageSourceAttachDataPtr data) { + qemuDomainStorageSourcePrivatePtr srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); + if (src->pr && !virStoragePRDefIsManaged(src->pr) && !(data->prmgrProps = qemuBuildPRManagerInfoProps(src))) return -1; + if (srcpriv) { + if (srcpriv->secinfo && + srcpriv->secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES && + qemuBuildSecretInfoProps(srcpriv->secinfo, &data->authsecretProps) < 0) + return -1; + + if (srcpriv->encinfo && + qemuBuildSecretInfoProps(srcpriv->encinfo, &data->encryptsecretProps) < 0) + return -1; + } + return 0; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 7ea5a531f3..d4db3d2bba 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -395,15 +395,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, virErrorPtr orig_err; char *devstr = NULL; char *managedPrmgrAlias = NULL; - char *encobjAlias = NULL; - char *secobjAlias = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); - virJSONValuePtr secobjProps = NULL; - virJSONValuePtr encobjProps = NULL; virJSONValuePtr managedPrmgrProps = NULL; - qemuDomainStorageSourcePrivatePtr srcPriv; - qemuDomainSecretInfoPtr secinfo = NULL; - qemuDomainSecretInfoPtr encinfo = NULL; if (qemuHotplugPrepareDiskAccess(driver, vm, disk, NULL, false) < 0) goto cleanup; @@ -414,20 +407,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (qemuDomainPrepareDiskSource(disk, priv, cfg) < 0) goto error; - srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); - if (srcPriv) { - secinfo = srcPriv->secinfo; - encinfo = srcPriv->encinfo; - } - - if (secinfo && secinfo->type == VIR_DOMAIN_SECRET_INFO_TYPE_AES) { - if (qemuBuildSecretInfoProps(secinfo, &secobjProps) < 0) - goto error; - } - - if (encinfo && qemuBuildSecretInfoProps(encinfo, &encobjProps) < 0) - goto error; - if (qemuDomainDiskAttachManagedPR(vm, disk, &managedPrmgrProps) < 0) goto error; @@ -449,14 +428,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); - if (secobjProps && - qemuMonitorAddObject(priv->mon, &secobjProps, &secobjAlias) < 0) - goto exit_monitor; - - if (encobjProps && - qemuMonitorAddObject(priv->mon, &encobjProps, &encobjAlias) < 0) - goto exit_monitor; - if (managedPrmgrProps && qemuMonitorAddObject(priv->mon, &managedPrmgrProps, &managedPrmgrAlias) < 0) goto exit_monitor; @@ -480,12 +451,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, cleanup: qemuBlockStorageSourceAttachDataFree(data); virJSONValueFree(managedPrmgrProps); - virJSONValueFree(encobjProps); - virJSONValueFree(secobjProps); qemuDomainSecretDiskDestroy(disk); VIR_FREE(managedPrmgrAlias); - VIR_FREE(secobjAlias); - VIR_FREE(encobjAlias); VIR_FREE(devstr); virObjectUnref(cfg); return ret; @@ -494,10 +461,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, qemuBlockStorageSourceAttachRollback(priv->mon, data); virErrorPreserveLast(&orig_err); - if (secobjAlias) - ignore_value(qemuMonitorDelObject(priv->mon, secobjAlias)); - if (encobjAlias) - ignore_value(qemuMonitorDelObject(priv->mon, encobjAlias)); if (managedPrmgrAlias) ignore_value(qemuMonitorDelObject(priv->mon, managedPrmgrAlias)); if (disk->src->tlsAlias) -- 2.16.2
On Fri, Jun 01, 2018 at 05:51:08PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 21 +++++++++++++++++++++ src/qemu/qemu_block.h | 6 ++++++ src/qemu/qemu_command.c | 13 +++++++++++++ src/qemu/qemu_hotplug.c | 37 ------------------------------------- 4 files changed, 40 insertions(+), 37 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 9 +++++++++ src/qemu/qemu_block.h | 3 +++ src/qemu/qemu_command.c | 9 ++++++++- src/qemu/qemu_command.h | 3 ++- src/qemu/qemu_hotplug.c | 37 +------------------------------------ 5 files changed, 23 insertions(+), 38 deletions(-) diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index b6b1316ea5..8053f08f6d 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1488,6 +1488,8 @@ qemuBlockStorageSourceAttachDataFree(qemuBlockStorageSourceAttachDataPtr data) virJSONValueFree(data->prmgrProps); virJSONValueFree(data->authsecretProps); virJSONValueFree(data->encryptsecretProps); + virJSONValueFree(data->tlsProps); + VIR_FREE(data->tlsAlias); VIR_FREE(data->authsecretAlias); VIR_FREE(data->encryptsecretAlias); VIR_FREE(data->driveCmd); @@ -1567,6 +1569,10 @@ qemuBlockStorageSourceAttachApply(qemuMonitorPtr mon, &data->encryptsecretAlias) < 0) return -1; + if (data->tlsProps && + qemuMonitorAddObject(mon, &data->tlsProps, &data->tlsAlias) < 0) + return -1; + if (data->storageProps) { rv = qemuMonitorBlockdevAdd(mon, data->storageProps); data->storageProps = NULL; @@ -1637,6 +1643,9 @@ qemuBlockStorageSourceAttachRollback(qemuMonitorPtr mon, if (data->encryptsecretAlias) ignore_value(qemuMonitorDelObject(mon, data->encryptsecretAlias)); + if (data->tlsAlias) + ignore_value(qemuMonitorDelObject(mon, data->tlsAlias)); + virErrorRestore(&orig_err); } diff --git a/src/qemu/qemu_block.h b/src/qemu/qemu_block.h index 4ffb42dfd6..418b5064b5 100644 --- a/src/qemu/qemu_block.h +++ b/src/qemu/qemu_block.h @@ -91,6 +91,9 @@ struct qemuBlockStorageSourceAttachData { virJSONValuePtr encryptsecretProps; char *encryptsecretAlias; + + virJSONValuePtr tlsProps; + char *tlsAlias; }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e11ae8b874..87b043d3f3 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -10469,13 +10469,15 @@ qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, * qemuBuildStorageSourceAttachPrepareCommon: * @src: storage source * @data: already initialized data for disk source addition + * @qemuCaps: qemu capabilities object * * Prepare data for configuration associated with the disk source such as * secrets/TLS/pr objects etc ... */ int qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, - qemuBlockStorageSourceAttachDataPtr data) + qemuBlockStorageSourceAttachDataPtr data, + virQEMUCapsPtr qemuCaps) { qemuDomainStorageSourcePrivatePtr srcpriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(src); @@ -10495,5 +10497,10 @@ qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, return -1; } + if (src->haveTLS == VIR_TRISTATE_BOOL_YES && + qemuBuildTLSx509BackendProps(src->tlsCertdir, false, true, src->tlsAlias, + NULL, qemuCaps, &data->tlsProps) < 0) + return -1; + return 0; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 711fce9648..0c2fdff807 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -108,7 +108,8 @@ qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, virQEMUCapsPtr qemuCaps); int qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, - qemuBlockStorageSourceAttachDataPtr data); + qemuBlockStorageSourceAttachDataPtr data, + virQEMUCapsPtr qemuCaps); /* Current, best practice */ char *qemuBuildDriveDevStr(const virDomainDef *def, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d4db3d2bba..d7c59b49c3 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -154,35 +154,6 @@ qemuHotplugPrepareDiskAccess(virQEMUDriverPtr driver, } -static int -qemuDomainAddDiskSrcTLSObject(virQEMUDriverPtr driver, - virDomainObjPtr vm, - virStorageSourcePtr src) -{ - int ret = -1; - qemuDomainObjPrivatePtr priv = vm->privateData; - virJSONValuePtr tlsProps = NULL; - - if (qemuDomainGetTLSObjects(priv->qemuCaps, NULL, - src->tlsCertdir, - false, true, - src->tlsAlias, - &tlsProps, NULL) < 0) - goto cleanup; - - if (qemuDomainAddTLSObjects(driver, vm, QEMU_ASYNC_JOB_NONE, - NULL, &tlsProps) < 0) - goto cleanup; - - ret = 0; - - cleanup: - virJSONValueFree(tlsProps); - - return ret; -} - - static int qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -413,11 +384,7 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (!(data = qemuBuildStorageSourceAttachPrepareDrive(disk, priv->qemuCaps))) goto error; - if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, data) < 0) - goto error; - - if (disk->src->haveTLS == VIR_TRISTATE_BOOL_YES && - qemuDomainAddDiskSrcTLSObject(driver, vm, disk->src) < 0) + if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, data, priv->qemuCaps) < 0) goto error; if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps))) @@ -463,8 +430,6 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, virErrorPreserveLast(&orig_err); if (managedPrmgrAlias) ignore_value(qemuMonitorDelObject(priv->mon, managedPrmgrAlias)); - if (disk->src->tlsAlias) - ignore_value(qemuMonitorDelObject(priv->mon, disk->src->tlsAlias)); if (qemuDomainObjExitMonitor(driver, vm) < 0) ret = -2; virErrorRestore(&orig_err); -- 2.16.2
On Fri, Jun 01, 2018 at 05:51:09PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_block.c | 9 +++++++++ src/qemu/qemu_block.h | 3 +++ src/qemu/qemu_command.c | 9 ++++++++- src/qemu/qemu_command.h | 3 ++- src/qemu/qemu_hotplug.c | 37 +------------------------------------ 5 files changed, 23 insertions(+), 38 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
It prepares all disk so use the plural form. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 87b043d3f3..118a412ffa 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2231,9 +2231,9 @@ qemuBuildDiskUnmanagedPRCommandLine(virCommandPtr cmd, static int -qemuBuildDiskDriveCommandLine(virCommandPtr cmd, - const virDomainDef *def, - virQEMUCapsPtr qemuCaps) +qemuBuildDisksCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virQEMUCapsPtr qemuCaps) { size_t i; unsigned int bootCD = 0; @@ -10096,7 +10096,7 @@ qemuBuildCommandLine(virQEMUDriverPtr driver, if (qemuBuildHubCommandLine(cmd, def, qemuCaps) < 0) goto error; - if (qemuBuildDiskDriveCommandLine(cmd, def, qemuCaps) < 0) + if (qemuBuildDisksCommandLine(cmd, def, qemuCaps) < 0) goto error; if (qemuBuildFSDevCommandLine(cmd, def, qemuCaps) < 0) -- 2.16.2
On Fri, Jun 01, 2018 at 05:51:10PM +0200, Peter Krempa wrote:
It prepares all disk so use the plural form.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 106 ++++++++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 44 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 118a412ffa..9fabfdcfef 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2230,6 +2230,64 @@ qemuBuildDiskUnmanagedPRCommandLine(virCommandPtr cmd, } +static int +qemuBuildDiskCommandLine(virCommandPtr cmd, + const virDomainDef *def, + virDomainDiskDefPtr disk, + virQEMUCapsPtr qemuCaps, + unsigned int bootindex, + bool driveBoot) +{ + char *optstr; + qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); + qemuDomainSecretInfoPtr secinfo = NULL; + qemuDomainSecretInfoPtr encinfo = NULL; + + if (srcPriv) { + secinfo = srcPriv->secinfo; + encinfo = srcPriv->encinfo; + } + + if (qemuBuildDiskUnmanagedPRCommandLine(cmd, disk->src) < 0) + return -1; + + if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0) + return -1; + + if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0) + return -1; + + if (qemuBuildDiskSrcTLSx509CommandLine(cmd, disk->src, qemuCaps) < 0) + return -1; + + virCommandAddArg(cmd, "-drive"); + + if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps))) + return -1; + + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + + if (qemuDiskBusNeedsDeviceArg(disk->bus)) { + if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { + if (qemuBulildFloppyCommandLineOptions(cmd, def, disk, + bootindex) < 0) + return -1; + } else { + virCommandAddArg(cmd, "-device"); + + if (!(optstr = qemuBuildDriveDevStr(def, disk, bootindex, + qemuCaps))) + return -1; + virCommandAddArg(cmd, optstr); + VIR_FREE(optstr); + } + } + + return 0; +} + + static int qemuBuildDisksCommandLine(virCommandPtr cmd, const virDomainDef *def, @@ -2260,18 +2318,9 @@ qemuBuildDisksCommandLine(virCommandPtr cmd, } for (i = 0; i < def->ndisks; i++) { - char *optstr; + virDomainDiskDefPtr disk = def->disks[i]; unsigned int bootindex = 0; bool driveBoot = false; - virDomainDiskDefPtr disk = def->disks[i]; - qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); - qemuDomainSecretInfoPtr secinfo = NULL; - qemuDomainSecretInfoPtr encinfo = NULL; - - if (srcPriv) { - secinfo = srcPriv->secinfo; - encinfo = srcPriv->encinfo; - } if (disk->info.bootIndex) { bootindex = disk->info.bootIndex; @@ -2297,42 +2346,11 @@ qemuBuildDisksCommandLine(virCommandPtr cmd, } } - if (qemuBuildDiskUnmanagedPRCommandLine(cmd, disk->src) < 0) - return -1; - - if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0) - return -1; - - if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0) - return -1; - - if (qemuBuildDiskSrcTLSx509CommandLine(cmd, disk->src, qemuCaps) < 0) + if (qemuBuildDiskCommandLine(cmd, def, disk, qemuCaps, + bootindex, driveBoot) < 0) return -1; - - virCommandAddArg(cmd, "-drive"); - - if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps))) - return -1; - - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - - if (qemuDiskBusNeedsDeviceArg(disk->bus)) { - if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { - if (qemuBulildFloppyCommandLineOptions(cmd, def, disk, - bootindex) < 0) - return -1; - } else { - virCommandAddArg(cmd, "-device"); - - if (!(optstr = qemuBuildDriveDevStr(def, disk, bootindex, - qemuCaps))) - return -1; - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); - } - } } + return 0; } -- 2.16.2
On Fri, Jun 01, 2018 at 05:51:11PM +0200, Peter Krempa wrote:
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 106 ++++++++++++++++++++++++++++-------------------- 1 file changed, 62 insertions(+), 44 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
Now that we have one place that sets up all disk-related objects to qemuBlockStorageSourceAttachDataPtr we can easily reuse the data in the command-line formatter by implementing a worker which will convert the data. A huge advantage is that it will be way easier to integrate this with -blockdev later on. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 94 +++++++++++++++++++------------------------------ src/qemu/qemu_command.h | 3 +- src/qemu/qemu_hotplug.c | 3 +- 3 files changed, 40 insertions(+), 60 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9fabfdcfef..e7d34703d0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -761,24 +761,6 @@ qemuBuildTLSx509CommandLine(virCommandPtr cmd, } -/* qemuBuildDiskSrcTLSx509CommandLine: - * - * Returns 0 on success, -1 w/ error on some sort of failure. - */ -static int -qemuBuildDiskSrcTLSx509CommandLine(virCommandPtr cmd, - virStorageSourcePtr src, - virQEMUCapsPtr qemuCaps) -{ - if (src->haveTLS != VIR_TRISTATE_BOOL_YES) - return 0; - - return qemuBuildTLSx509CommandLine(cmd, src->tlsCertdir, - false, true, - NULL, src->tlsAlias, qemuCaps); -} - - static char * qemuBuildNetworkDriveURI(virStorageSourcePtr src, qemuDomainSecretInfoPtr secinfo) @@ -2202,31 +2184,40 @@ qemuBulildFloppyCommandLineOptions(virCommandPtr cmd, static int -qemuBuildDiskUnmanagedPRCommandLine(virCommandPtr cmd, - virStorageSourcePtr src) +qemuBuildObjectCommandline(virCommandPtr cmd, + virJSONValuePtr objProps) { virBuffer buf = VIR_BUFFER_INITIALIZER; - virJSONValuePtr props = NULL; - int ret = -1; - if (!src->pr || - virStoragePRDefIsManaged(src->pr)) + if (!objProps) return 0; - if (!(props = qemuBuildPRManagerInfoProps(src))) + if (virQEMUBuildObjectCommandlineFromJSON(&buf, objProps) < 0) { + virBufferFreeAndReset(&buf); return -1; - - if (virQEMUBuildObjectCommandlineFromJSON(&buf, props) < 0) - goto cleanup; + } virCommandAddArg(cmd, "-object"); virCommandAddArgBuffer(cmd, &buf); - ret = 0; - cleanup: - virBufferFreeAndReset(&buf); - virJSONValueFree(props); - return ret; + return 0; +} + + +static int +qemuBuildBlockStorageSourceAttachDataCommandline(virCommandPtr cmd, + qemuBlockStorageSourceAttachDataPtr data) +{ + if (qemuBuildObjectCommandline(cmd, data->prmgrProps) < 0 || + qemuBuildObjectCommandline(cmd, data->authsecretProps) < 0 || + qemuBuildObjectCommandline(cmd, data->encryptsecretProps) < 0 || + qemuBuildObjectCommandline(cmd, data->tlsProps) < 0) + return -1; + + if (data->driveCmd) + virCommandAddArgList(cmd, "-drive", data->driveCmd, NULL); + + return 0; } @@ -2238,35 +2229,20 @@ qemuBuildDiskCommandLine(virCommandPtr cmd, unsigned int bootindex, bool driveBoot) { + qemuBlockStorageSourceAttachDataPtr data = NULL; char *optstr; - qemuDomainStorageSourcePrivatePtr srcPriv = QEMU_DOMAIN_STORAGE_SOURCE_PRIVATE(disk->src); - qemuDomainSecretInfoPtr secinfo = NULL; - qemuDomainSecretInfoPtr encinfo = NULL; - - if (srcPriv) { - secinfo = srcPriv->secinfo; - encinfo = srcPriv->encinfo; - } - - if (qemuBuildDiskUnmanagedPRCommandLine(cmd, disk->src) < 0) - return -1; - - if (qemuBuildDiskSecinfoCommandLine(cmd, secinfo) < 0) - return -1; - - if (qemuBuildDiskSecinfoCommandLine(cmd, encinfo) < 0) - return -1; - if (qemuBuildDiskSrcTLSx509CommandLine(cmd, disk->src, qemuCaps) < 0) + if (!(data = qemuBuildStorageSourceAttachPrepareDrive(disk, qemuCaps, + driveBoot))) return -1; - virCommandAddArg(cmd, "-drive"); - - if (!(optstr = qemuBuildDriveStr(disk, driveBoot, qemuCaps))) + if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, data, qemuCaps) < 0 || + qemuBuildBlockStorageSourceAttachDataCommandline(cmd, data) < 0) { + qemuBlockStorageSourceAttachDataFree(data); return -1; + } - virCommandAddArg(cmd, optstr); - VIR_FREE(optstr); + qemuBlockStorageSourceAttachDataFree(data); if (qemuDiskBusNeedsDeviceArg(disk->bus)) { if (disk->bus == VIR_DOMAIN_DISK_BUS_FDC) { @@ -10460,20 +10436,22 @@ qemuBuildHotpluggableCPUProps(const virDomainVcpuDef *vcpu) * qemuBuildStorageSourceAttachPrepareDrive: * @disk: disk object to prepare * @qemuCaps: qemu capabilities object + * @driveBoot: bootable flag for disks which don't have -device part * * Prepare qemuBlockStorageSourceAttachDataPtr for use with the old approach * using -drive/drive_add. See qemuBlockStorageSourceAttachPrepareBlockdev. */ qemuBlockStorageSourceAttachDataPtr qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, - virQEMUCapsPtr qemuCaps) + virQEMUCapsPtr qemuCaps, + bool driveBoot) { qemuBlockStorageSourceAttachDataPtr data = NULL; if (VIR_ALLOC(data) < 0) return NULL; - if (!(data->driveCmd = qemuBuildDriveStr(disk, false, qemuCaps)) || + if (!(data->driveCmd = qemuBuildDriveStr(disk, driveBoot, qemuCaps)) || !(data->driveAlias = qemuAliasDiskDriveFromDisk(disk))) { qemuBlockStorageSourceAttachDataFree(data); return NULL; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 0c2fdff807..3a13c3d5f8 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -105,7 +105,8 @@ char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk); qemuBlockStorageSourceAttachDataPtr qemuBuildStorageSourceAttachPrepareDrive(virDomainDiskDefPtr disk, - virQEMUCapsPtr qemuCaps); + virQEMUCapsPtr qemuCaps, + bool driveBoot); int qemuBuildStorageSourceAttachPrepareCommon(virStorageSourcePtr src, qemuBlockStorageSourceAttachDataPtr data, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index d7c59b49c3..7c4e9766d7 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -381,7 +381,8 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, if (qemuDomainDiskAttachManagedPR(vm, disk, &managedPrmgrProps) < 0) goto error; - if (!(data = qemuBuildStorageSourceAttachPrepareDrive(disk, priv->qemuCaps))) + if (!(data = qemuBuildStorageSourceAttachPrepareDrive(disk, priv->qemuCaps, + false))) goto error; if (qemuBuildStorageSourceAttachPrepareCommon(disk->src, data, priv->qemuCaps) < 0) -- 2.16.2
On Fri, Jun 01, 2018 at 05:51:12PM +0200, Peter Krempa wrote:
Now that we have one place that sets up all disk-related objects to qemuBlockStorageSourceAttachDataPtr we can easily reuse the data in the command-line formatter by implementing a worker which will convert the data.
A huge advantage is that it will be way easier to integrate this with -blockdev later on.
Sadly, I might not live long enough to see that.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_command.c | 94 +++++++++++++++++++------------------------------ src/qemu/qemu_command.h | 3 +- src/qemu/qemu_hotplug.c | 3 +- 3 files changed, 40 insertions(+), 60 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Peter Krempa