4:37 p.m.
A post 2.0.0 type adjustment...
While answering a question for bz 1289391 regarding the USB path - I figured
that making the code be more the virtio and scsi code paths would at least
have a better look'n'feel. Then I tripped in to the alias morass and cleaned
that up a bit too.
John Ferlan (5):
qemu: Reorder qemuDomainAttachUSBMassStorageDevice failure path
qemu: Move drive alias processing to qemu_alias
qemu: Use qemuAssignDeviceDiskDriveAlias
qemu: Make QEMU_DRIVE_HOST_PREFIX more private
qemu: Add attempt to call qemuMonitorDriveDel for USB failure path
src/qemu/qemu_alias.c | 34 +++++++++++++++++++++++++
src/qemu/qemu_alias.h | 4 +++
src/qemu/qemu_command.c | 23 ++++++++---------
src/qemu/qemu_command.h | 1 -
src/qemu/qemu_domain.c | 3 +--
src/qemu/qemu_driver.c | 3 +--
src/qemu/qemu_hotplug.c | 59 ++++++++++++++++++++++++++++----------------
src/qemu/qemu_migration.c | 11 +++------
src/qemu/qemu_monitor_json.c | 10 +++-----
src/qemu/qemu_monitor_text.c | 18 ++++++++------
src/qemu/qemu_process.c | 3 +--
11 files changed, 107 insertions(+), 62 deletions(-)
--
2.5.5
4:38 p.m.
New subject: [libvirt] [PATCH 1/5] qemu: Reorder qemuDomainAttachUSBMassStorageDevice failure path
Modify the error/exit path to match what was done for Virtio and SCSI.
If nothing else it'll have a consistent look'n'feel
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 39 ++++++++++++++++++++++-----------------
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index e0b8230..ae5203e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -649,27 +649,20 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
goto error;
qemuDomainObjEnterMonitor(driver, vm);
- ret = qemuMonitorAddDrive(priv->mon, drivestr);
- if (ret == 0) {
- ret = qemuMonitorAddDevice(priv->mon, devstr);
- if (ret < 0) {
- VIR_WARN("qemuMonitorAddDevice failed on %s (%s)",
- drivestr, devstr);
- /* XXX should call 'drive_del' on error but this does not
- exist yet */
- }
- }
- if (qemuDomainObjExitMonitor(driver, vm) < 0) {
- ret = -1;
- goto error;
- }
- virDomainAuditDisk(vm, NULL, disk->src, "attach", ret >= 0);
+ if (qemuMonitorAddDrive(priv->mon, drivestr) < 0)
+ goto exit_monitor;
- if (ret < 0)
- goto error;
+ if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
+ goto failadddevice;
+
+ if (qemuDomainObjExitMonitor(driver, vm) < 0)
+ goto failexitmonitor;
+
+ virDomainAuditDisk(vm, NULL, disk->src, "attach", true);
virDomainDiskInsertPreAlloced(vm->def, disk);
+ ret = 0;
cleanup:
VIR_FREE(devstr);
@@ -677,6 +670,18 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
virObjectUnref(cfg);
return ret;
+ failadddevice:
+ VIR_WARN("qemuMonitorAddDevice failed on %s (%s)",
+ drivestr, devstr);
+ /* XXX should call 'drive_del' on error but this does not
+ exist yet */
+
+ exit_monitor:
+ ignore_value(qemuDomainObjExitMonitor(driver, vm));
+
+ failexitmonitor:
+ virDomainAuditDisk(vm, NULL, disk->src, "attach", false);
+
error:
ignore_value(qemuDomainPrepareDisk(driver, vm, disk, NULL, true));
goto cleanup;
--
2.5.5
4:38 p.m.
New subject: [libvirt] [PATCH 2/5] qemu: Move drive alias processing to qemu_alias
Move qemuDeviceDriveHostAlias from qemu_command and rename to
qemuAssignDeviceDiskDriveAlias in qemu_alias. Also change the parameter
to just be the disk->info.alias (e.g. srcalias)
Includes some innocent bystanders that seem to need QEMU_DRIVE_HOST_PREFIX
which requires include qemu_alias.h instead of qemu_command.h
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_alias.c | 17 +++++++++++++++++
src/qemu/qemu_alias.h | 4 ++++
src/qemu/qemu_command.c | 11 -----------
src/qemu/qemu_command.h | 1 -
src/qemu/qemu_hotplug.c | 4 ++--
src/qemu/qemu_migration.c | 2 +-
src/qemu/qemu_monitor_json.c | 2 +-
src/qemu/qemu_monitor_text.c | 2 +-
8 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index d624071..b4a6e52 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -170,6 +170,23 @@ qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef,
return virAsprintf(&controller->info.alias, "%s%d", prefix,
controller->idx);
}
+/* qemuAssignDeviceDiskDriveAlias:
+ * @srcalias: Disk source alias
+ *
+ * Generate and return an alias for the device disk '-drive'
+ *
+ * Returns NULL or a string containing the alias
+ */
+char *
+qemuAssignDeviceDiskDriveAlias(const char *srcalias)
+{
+ char *ret;
+
+ if (virAsprintf(&ret, "%s%s", QEMU_DRIVE_HOST_PREFIX, srcalias) <
0)
+ return NULL;
+ return ret;
+}
+
/* Our custom -drive naming scheme used with id= */
int
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index e328a9b..972de55 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -30,6 +30,8 @@
# include "qemu_domain.h"
# include "qemu_domain_address.h"
+# define QEMU_DRIVE_HOST_PREFIX "drive-"
+
int qemuAssignDeviceChrAlias(virDomainDefPtr def,
virDomainChrDefPtr chr,
ssize_t idx);
@@ -38,6 +40,8 @@ int qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef,
virQEMUCapsPtr qemuCaps,
virDomainControllerDefPtr controller);
+char *qemuAssignDeviceDiskDriveAlias(const char *srcalias);
+
int qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef,
virDomainDiskDefPtr def,
virQEMUCapsPtr qemuCaps);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 71e9e63..c42ae16 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -287,17 +287,6 @@ qemuVirCommandGetDevSet(virCommandPtr cmd, int fd)
}
-char *qemuDeviceDriveHostAlias(virDomainDiskDefPtr disk)
-{
- char *ret;
-
- if (virAsprintf(&ret, "%s%s",
- QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0)
- return NULL;
- return ret;
-}
-
-
static int
qemuBuildDeviceAddressStr(virBufferPtr buf,
const virDomainDef *domainDef,
diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h
index c4d0567..dcf9ba6 100644
--- a/src/qemu/qemu_command.h
+++ b/src/qemu/qemu_command.h
@@ -37,7 +37,6 @@
/* Config type for XML import/export conversions */
# define QEMU_CONFIG_FORMAT_ARGV "qemu-argv"
-# define QEMU_DRIVE_HOST_PREFIX "drive-"
# define QEMU_FSDEV_HOST_PREFIX "fsdev-"
# define QEMU_BLOCK_IOTUNE_MAX 1000000000000000LL
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index ae5203e..612b6ec 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -229,7 +229,7 @@ qemuDomainChangeEjectableMedia(virQEMUDriverPtr driver,
if (qemuDomainPrepareDisk(driver, vm, disk, newsrc, false) < 0)
goto cleanup;
- if (!(driveAlias = qemuDeviceDriveHostAlias(disk)))
+ if (!(driveAlias = qemuAssignDeviceDiskDriveAlias(disk->info.alias)))
goto error;
qemuDomainObjEnterMonitor(driver, vm);
@@ -345,7 +345,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn,
if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
goto error;
- if (!(drivealias = qemuDeviceDriveHostAlias(disk)))
+ if (!(drivealias = qemuAssignDeviceDiskDriveAlias(disk->info.alias)))
goto error;
if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index c411dab..25dabb8 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -36,7 +36,7 @@
#include "qemu_domain.h"
#include "qemu_process.h"
#include "qemu_capabilities.h"
-#include "qemu_command.h"
+#include "qemu_alias.h"
#include "qemu_cgroup.h"
#include "qemu_hotplug.h"
#include "qemu_blockjob.h"
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bb426dc..a54ff8d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -33,7 +33,7 @@
#include "qemu_monitor_text.h"
#include "qemu_monitor_json.h"
-#include "qemu_command.h"
+#include "qemu_alias.h"
#include "qemu_parse_command.h"
#include "qemu_capabilities.h"
#include "viralloc.h"
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 9295219..6c458e2 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -31,7 +31,7 @@
#include <string.h>
#include "qemu_monitor_text.h"
-#include "qemu_command.h"
+#include "qemu_alias.h"
#include "c-ctype.h"
#include "c-strcasestr.h"
#include "viralloc.h"
--
2.5.5
4:38 p.m.
New subject: [libvirt] [PATCH 3/5] qemu: Use qemuAssignDeviceDiskDriveAlias
Rather than open code build the drive alias command in multiple places,
use the helper to ensure consistency.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_command.c | 12 ++++++++++--
src/qemu/qemu_driver.c | 3 +--
src/qemu/qemu_hotplug.c | 3 +--
src/qemu/qemu_migration.c | 9 +++------
src/qemu/qemu_monitor_json.c | 2 +-
src/qemu/qemu_monitor_text.c | 8 ++++++--
6 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index c42ae16..a11ebd0 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1251,7 +1251,11 @@ qemuBuildDriveStr(virDomainDiskDefPtr disk,
}
if (emitDeviceSyntax) {
- virBufferAsprintf(&opt, ",id=%s%s", QEMU_DRIVE_HOST_PREFIX,
disk->info.alias);
+ char *drivealias = qemuAssignDeviceDiskDriveAlias(disk->info.alias);
+ if (!drivealias)
+ goto error;
+ virBufferAsprintf(&opt, ",id=%s", drivealias);
+ VIR_FREE(drivealias);
} else {
if (busid == -1 && unitid == -1) {
if (idx != -1)
@@ -1613,6 +1617,7 @@ qemuBuildDriveDevStr(const virDomainDef *def,
virBuffer opt = VIR_BUFFER_INITIALIZER;
const char *bus = virDomainDiskQEMUBusTypeToString(disk->bus);
const char *contAlias;
+ char *drivealias;
int controllerModel;
if (qemuCheckDiskConfig(disk) < 0)
@@ -1838,7 +1843,10 @@ qemuBuildDriveDevStr(const virDomainDef *def,
goto error;
}
- virBufferAsprintf(&opt, ",drive=%s%s", QEMU_DRIVE_HOST_PREFIX,
disk->info.alias);
+ if (!(drivealias = qemuAssignDeviceDiskDriveAlias(disk->info.alias)))
+ goto error;
+ virBufferAsprintf(&opt, ",drive=%s", drivealias);
+ VIR_FREE(drivealias);
virBufferAsprintf(&opt, ",id=%s", disk->info.alias);
if (bootindex && virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOTINDEX))
virBufferAsprintf(&opt, ",bootindex=%u", bootindex);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 61d184b..a65b299 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -10353,8 +10353,7 @@ qemuDomainBlockResize(virDomainPtr dom,
disk->src->format == VIR_STORAGE_FILE_QED)
size = VIR_ROUND_UP(size, 512);
- if (virAsprintf(&device, "%s%s", QEMU_DRIVE_HOST_PREFIX,
- disk->info.alias) < 0)
+ if (!(device = qemuAssignDeviceDiskDriveAlias(disk->info.alias)))
goto endjob;
qemuDomainObjEnterMonitor(driver, vm);
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 612b6ec..be3ba4d 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2798,8 +2798,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 (virAsprintf(&drivestr, "%s%s",
- QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0)
+ if (!(drivestr = qemuAssignDeviceDiskDriveAlias(disk->info.alias)))
return -1;
qemuDomainObjEnterMonitor(driver, vm);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 25dabb8..da9e710 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1761,8 +1761,7 @@ qemuMigrationStartNBDServer(virQEMUDriverPtr driver,
continue;
VIR_FREE(diskAlias);
- if (virAsprintf(&diskAlias, "%s%s",
- QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0)
+ if (!(diskAlias = qemuAssignDeviceDiskDriveAlias(disk->info.alias)))
goto cleanup;
if (qemuDomainObjEnterMonitorAsync(driver, vm,
@@ -1978,8 +1977,7 @@ qemuMigrationCancelOneDriveMirror(virQEMUDriverPtr driver,
return 1;
}
- if (virAsprintf(&diskAlias, "%s%s",
- QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0)
+ if (!(diskAlias = qemuAssignDeviceDiskDriveAlias(disk->info.alias)))
return -1;
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) < 0)
@@ -2154,8 +2152,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
if (!qemuMigrateDisk(disk, nmigrate_disks, migrate_disks))
continue;
- if ((virAsprintf(&diskAlias, "%s%s",
- QEMU_DRIVE_HOST_PREFIX, disk->info.alias) < 0) ||
+ if (!(diskAlias = qemuAssignDeviceDiskDriveAlias(disk->info.alias)) ||
(virAsprintf(&nbd_dest, "nbd:%s:%d:exportname=%s",
hoststr, port, diskAlias) < 0))
goto cleanup;
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a54ff8d..633fb2a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3686,7 +3686,7 @@ int qemuMonitorJSONSetDrivePassphrase(qemuMonitorPtr mon,
virJSONValuePtr reply = NULL;
char *drive;
- if (virAsprintf(&drive, "%s%s", QEMU_DRIVE_HOST_PREFIX, alias) < 0)
+ if (!(drive = qemuAssignDeviceDiskDriveAlias(alias)))
return -1;
cmd = qemuMonitorJSONMakeCommand("block_passwd",
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index 6c458e2..ff92bb1 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -2001,6 +2001,7 @@ int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon,
{
char *cmd = NULL;
char *reply = NULL;
+ char *drivealias = NULL;
int ret = -1;
char *safe_str;
@@ -2008,8 +2009,10 @@ int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon,
if (!safe_str)
return -1;
- if (virAsprintf(&cmd, "block_passwd %s%s \"%s\"",
- QEMU_DRIVE_HOST_PREFIX, alias, safe_str) < 0)
+ if (!(drivealias = qemuAssignDeviceDiskDriveAlias(alias)))
+ goto cleanup;
+
+ if (virAsprintf(&cmd, "block_passwd %s \"%s\"", drivealias,
safe_str) < 0)
goto cleanup;
if (qemuMonitorHMPCommand(mon, cmd, &reply) < 0)
@@ -2030,6 +2033,7 @@ int qemuMonitorTextSetDrivePassphrase(qemuMonitorPtr mon,
cleanup:
VIR_FREE(cmd);
VIR_FREE(reply);
+ VIR_FREE(drivealias);
VIR_FREE(safe_str);
return ret;
}
--
2.5.5
4:38 p.m.
New subject: [libvirt] [PATCH 4/5] qemu: Make QEMU_DRIVE_HOST_PREFIX more private
Move QEMU_DRIVE_HOST_PREFIX into the qemu_alias.c to disuade future
callers from using it. Create qemuAliasDeviceDiskDriveSkipPrefix in
order to handle the current consumers that desire to check if an alias
has the drive- prefix and "get beyond it" in order to get the disk alias.
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_alias.c | 17 +++++++++++++++++
src/qemu/qemu_alias.h | 4 ++--
src/qemu/qemu_domain.c | 3 +--
src/qemu/qemu_monitor_json.c | 6 ++----
src/qemu/qemu_monitor_text.c | 8 +++-----
src/qemu/qemu_process.c | 3 +--
6 files changed, 26 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index b4a6e52..c7ef45a 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -29,6 +29,8 @@
#include "virstring.h"
#include "network/bridge_driver.h"
+#define QEMU_DRIVE_HOST_PREFIX "drive-"
+
#define VIR_FROM_THIS VIR_FROM_QEMU
VIR_LOG_INIT("qemu.qemu_alias");
@@ -188,6 +190,21 @@ qemuAssignDeviceDiskDriveAlias(const char *srcalias)
}
+/* qemuAliasDeviceDiskDriveSkipPrefix:
+ * @dev_name: Pointer to a const char string
+ *
+ * If the QEMU_DRIVE_HOST_PREFIX exists in the input string, then
+ * increment the pointer and return it
+ */
+const char *
+qemuAliasDeviceDiskDriveSkipPrefix(const char *dev_name)
+{
+ if (STRPREFIX(dev_name, QEMU_DRIVE_HOST_PREFIX))
+ dev_name += strlen(QEMU_DRIVE_HOST_PREFIX);
+ return dev_name;
+}
+
+
/* Our custom -drive naming scheme used with id= */
int
qemuAssignDeviceDiskAlias(virDomainDefPtr def,
diff --git a/src/qemu/qemu_alias.h b/src/qemu/qemu_alias.h
index 972de55..31bab07 100644
--- a/src/qemu/qemu_alias.h
+++ b/src/qemu/qemu_alias.h
@@ -30,8 +30,6 @@
# include "qemu_domain.h"
# include "qemu_domain_address.h"
-# define QEMU_DRIVE_HOST_PREFIX "drive-"
-
int qemuAssignDeviceChrAlias(virDomainDefPtr def,
virDomainChrDefPtr chr,
ssize_t idx);
@@ -42,6 +40,8 @@ int qemuAssignDeviceControllerAlias(virDomainDefPtr domainDef,
char *qemuAssignDeviceDiskDriveAlias(const char *srcalias);
+const char *qemuAliasDeviceDiskDriveSkipPrefix(const char *dev_name);
+
int qemuAssignDeviceDiskAlias(virDomainDefPtr vmdef,
virDomainDiskDefPtr def,
virQEMUCapsPtr qemuCaps);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index fd25669..3264b94 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4225,8 +4225,7 @@ qemuDomainStorageAlias(const char *device, int depth)
{
char *alias;
- if (STRPREFIX(device, QEMU_DRIVE_HOST_PREFIX))
- device += strlen(QEMU_DRIVE_HOST_PREFIX);
+ device = qemuAliasDeviceDiskDriveSkipPrefix(device);
if (!depth)
ignore_value(VIR_STRDUP(alias, device));
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 633fb2a..4be4c6e 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1796,8 +1796,7 @@ int qemuMonitorJSONGetBlockInfo(qemuMonitorPtr mon,
goto cleanup;
}
- if (STRPREFIX(thisdev, QEMU_DRIVE_HOST_PREFIX))
- thisdev += strlen(QEMU_DRIVE_HOST_PREFIX);
+ thisdev = qemuAliasDeviceDiskDriveSkipPrefix(thisdev);
if (VIR_ALLOC(info) < 0)
goto cleanup;
@@ -4196,8 +4195,7 @@ qemuMonitorJSONParseBlockJobInfo(virHashTablePtr blockJobs,
_("entry was missing 'device'"));
return -1;
}
- if (STRPREFIX(device, QEMU_DRIVE_HOST_PREFIX))
- device += strlen(QEMU_DRIVE_HOST_PREFIX);
+ device = qemuAliasDeviceDiskDriveSkipPrefix(device);
if (VIR_ALLOC(info) < 0 ||
virHashAddEntry(blockJobs, device, info) < 0) {
diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
index ff92bb1..b924ce6 100644
--- a/src/qemu/qemu_monitor_text.c
+++ b/src/qemu/qemu_monitor_text.c
@@ -753,8 +753,7 @@ int qemuMonitorTextGetBlockInfo(qemuMonitorPtr mon,
p = reply;
while (*p) {
- if (STRPREFIX(p, QEMU_DRIVE_HOST_PREFIX))
- p += strlen(QEMU_DRIVE_HOST_PREFIX);
+ p = (char *)qemuAliasDeviceDiskDriveSkipPrefix(p);
eol = strchr(p, '\n');
if (!eol)
@@ -839,7 +838,7 @@ qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon,
{
qemuBlockStatsPtr stats = NULL;
char *info = NULL;
- char *dev_name;
+ const char *dev_name;
char **lines = NULL;
char **values = NULL;
char *line;
@@ -901,8 +900,7 @@ qemuMonitorTextGetAllBlockStatsInfo(qemuMonitorPtr mon,
*line = '\0';
line += 2;
- if (STRPREFIX(dev_name, QEMU_DRIVE_HOST_PREFIX))
- dev_name += strlen(QEMU_DRIVE_HOST_PREFIX);
+ dev_name = qemuAliasDeviceDiskDriveSkipPrefix(dev_name);
if (!(values = virStringSplit(line, " ", 0)))
goto cleanup;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 63da600..a4adeac 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -354,8 +354,7 @@ qemuProcessFindDomainDiskByAlias(virDomainObjPtr vm,
{
size_t i;
- if (STRPREFIX(alias, QEMU_DRIVE_HOST_PREFIX))
- alias += strlen(QEMU_DRIVE_HOST_PREFIX);
+ alias = qemuAliasDeviceDiskDriveSkipPrefix(alias);
for (i = 0; i < vm->def->ndisks; i++) {
virDomainDiskDefPtr disk;
--
2.5.5
4:38 p.m.
New subject: [libvirt] [PATCH 5/5] qemu: Add attempt to call qemuMonitorDriveDel for USB failure path
Similar to the other disk types, add the qemuMonitorDriveDel in the failure
to add/hotplug a USB.
Added a couple of other formatting changes just to have a less cluttered look
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/qemu/qemu_hotplug.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index be3ba4d..f73a0a8 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -622,7 +622,9 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
virDomainDiskDefPtr disk)
{
qemuDomainObjPrivatePtr priv = vm->privateData;
+ virErrorPtr orig_err;
int ret = -1;
+ char *drivealias = NULL;
char *drivestr = NULL;
char *devstr = NULL;
virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
@@ -640,8 +642,13 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
if (qemuAssignDeviceDiskAlias(vm->def, disk, priv->qemuCaps) < 0)
goto error;
+
if (!(drivestr = qemuBuildDriveStr(disk, false, priv->qemuCaps)))
goto error;
+
+ if (!(drivealias = qemuAssignDeviceDiskDriveAlias(disk->info.alias)))
+ goto error;
+
if (!(devstr = qemuBuildDriveDevStr(vm->def, disk, 0, priv->qemuCaps)))
goto error;
@@ -666,15 +673,21 @@ qemuDomainAttachUSBMassStorageDevice(virQEMUDriverPtr driver,
cleanup:
VIR_FREE(devstr);
+ VIR_FREE(drivealias);
VIR_FREE(drivestr);
virObjectUnref(cfg);
return ret;
failadddevice:
- VIR_WARN("qemuMonitorAddDevice failed on %s (%s)",
- drivestr, devstr);
- /* XXX should call 'drive_del' on error but this does not
- exist yet */
+ orig_err = virSaveLastError();
+ if (qemuMonitorDriveDel(priv->mon, drivealias) < 0) {
+ VIR_WARN("Unable to remove drive %s (%s) after failed "
+ "qemuMonitorAddDevice", drivealias, drivestr);
+ }
+ if (orig_err) {
+ virSetError(orig_err);
+ virFreeError(orig_err);
+ }
exit_monitor:
ignore_value(qemuDomainObjExitMonitor(driver, vm));
--
2.5.5
10:14 a.m.
On 06/29/2016 05:37 PM, John Ferlan wrote:
A post 2.0.0 type adjustment...
While answering a question for bz 1289391 regarding the USB path - I figured
that making the code be more the virtio and scsi code paths would at least
have a better look'n'feel. Then I tripped in to the alias morass and cleaned
that up a bit too.
John Ferlan (5):
qemu: Reorder qemuDomainAttachUSBMassStorageDevice failure path
qemu: Move drive alias processing to qemu_alias
qemu: Use qemuAssignDeviceDiskDriveAlias
qemu: Make QEMU_DRIVE_HOST_PREFIX more private
qemu: Add attempt to call qemuMonitorDriveDel for USB failure path
Note there is https://bugzilla.redhat.com/show_bug.cgi?id=1336225 covering
some of these changes. There's also a missing drive_del call in AttachSCSI.
And in fact the whole drive_add, device_add, and unwinding on failure can
probably be shared between AttachVirtio/AttachUSB/AttachSCSI . Not saying it
blocks this series, but there's more cleanup in this area if you're motivated :)
Thanks,
Cole
3:36 p.m.
On 06/30/2016 11:14 AM, Cole Robinson wrote:
On 06/29/2016 05:37 PM, John Ferlan wrote:
> A post 2.0.0 type adjustment...
>
> While answering a question for bz 1289391 regarding the USB path - I figured
> that making the code be more the virtio and scsi code paths would at least
> have a better look'n'feel. Then I tripped in to the alias morass and
cleaned
> that up a bit too.
>
> John Ferlan (5):
> qemu: Reorder qemuDomainAttachUSBMassStorageDevice failure path
> qemu: Move drive alias processing to qemu_alias
> qemu: Use qemuAssignDeviceDiskDriveAlias
> qemu: Make QEMU_DRIVE_HOST_PREFIX more private
> qemu: Add attempt to call qemuMonitorDriveDel for USB failure path
>
Note there is https://bugzilla.redhat.com/show_bug.cgi?id=1336225 covering
some of these changes. There's also a missing drive_del call in AttachSCSI.
And in fact the whole drive_add, device_add, and unwinding on failure can
probably be shared between AttachVirtio/AttachUSB/AttachSCSI . Not saying it
blocks this series, but there's more cleanup in this area if you're motivated :)
I thought about touching AttachSCSI, but I knew I still had changes from
the pending LUKS secret series and I didn't want to have to continue
messing with it until that got pushed.
And yes, as I was going through this I started thinking - there's a lot
of similar code that perhaps could be combined. One thing at a time I
guess though.
Thanks!
John
3065
days inactive
3067
days old
7 comments
2 participants
participants (2)
-
Cole Robinson -
John Ferlan