$SUBJ: qemu: Add some more details for hotplug errors when device not found On 01/05/2018 05:28 AM, Chen Hanxiao wrote:

From: Chen Hanxiao <chenhanxiao@gmail.com>

More proper/detail error messages updated.> Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- src/libvirt_private.syms | 2 ++ src/qemu/qemu_hotplug.c | 42 +++++++++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a705fa846..512314484 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -203,6 +203,7 @@ virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; virDomainChrDefFree; virDomainChrDefNew; +virDomainChrDeviceTypeToString; virDomainChrEquals; virDomainChrFind; virDomainChrGetDomainPtrs; @@ -427,6 +428,7 @@ virDomainMemoryDefFree; virDomainMemoryFindByDef; virDomainMemoryFindInactiveByDef; virDomainMemoryInsert; +virDomainMemoryModelTypeToString; virDomainMemoryRemove; virDomainMemorySourceTypeFromString; virDomainMemorySourceTypeToString; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 385be80f2..6472a13a8 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3513,8 +3513,9 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, int ret = -1;

if (!olddev) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("cannot find existing graphics device to modify")); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("cannot find existing graphics device to modify of" + " type '%s'"), type);

Should be "modify of " and "type '%s'" IOW: The space goes on the 1st line...

goto cleanup; }

@@ -5050,8 +5051,10 @@ qemuDomainDetachShmemDevice(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData;

if ((idx = virDomainShmemDefFind(vm->def, dev)) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("device not present in domain configuration")); + virReportError(VIR_ERR_OPERATION_INVALID, + _("shmem device of model '%s' not found " + "in domain configuration"),

How about "model '%s' shmem device not present in domain configuration".

+ virDomainShmemModelTypeToString(dev->model)); return -1; }

@@ -5107,8 +5110,10 @@ qemuDomainDetachWatchdog(virQEMUDriverPtr driver, watchdog->model == dev->model && watchdog->action == dev->action && virDomainDeviceInfoAddressIsEqual(&dev->info, &watchdog->info))) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("watchdog device not present in domain configuration")); + virReportError(VIR_ERR_OPERATION_INVALID, + _("watchdog device of model '%s' is not " + "found in domain configuration"), + virDomainWatchdogModelTypeToString(watchdog->model));

"model '%s' watchdog device not present in domain configuration"

return -1; }

@@ -5148,8 +5153,13 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver, virDomainNetDefPtr detach = NULL; qemuDomainObjPrivatePtr priv = vm->privateData;

- if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0) + if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0) { + char mac[VIR_MAC_STRING_BUFLEN]; + virReportError(VIR_ERR_INTERNAL_ERROR, + _("netdev '%s' not found in domain configuration"), + virMacAddrFormat(&dev->data.net->mac, mac));

This could override some other message from virDomainNetFindIdx. I think you should look there to see messages that perhaps could utilize the new message from the next patch. Thus we drop this one, but have to modify virDomainNetFindIdx in the next patch to use the new error message.

goto cleanup; + }

detach = vm->def->nets[detachidx];

@@ -5335,8 +5345,10 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, char *devstr = NULL;

if (!(tmpChr = virDomainChrFind(vmdef, chr))) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("device not present in domain configuration")); + virReportError(VIR_ERR_OPERATION_INVALID, + _("chr device of type '%s' not found " + "in domain configuration"), + virDomainChrDeviceTypeToString(chr->deviceType));

"chr type '%s' device not present in domain configuration"

goto cleanup; }

@@ -5382,8 +5394,10 @@ qemuDomainDetachRNGDevice(virQEMUDriverPtr driver, int ret = -1;

if ((idx = virDomainRNGFind(vm->def, rng)) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("device not present in domain configuration")); + virReportError(VIR_ERR_OPERATION_INVALID, + _("RNG device of model '%s' not found " + "in domain configuration"), + virDomainRNGBackendTypeToString(rng->model));

"model '%s' RNG device not present in domain configuration"

return -1; }

@@ -5425,8 +5439,10 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver, qemuDomainMemoryDeviceAlignSize(vm->def, memdef);

if ((idx = virDomainMemoryFindByDef(vm->def, memdef)) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, "%s", - _("device not present in domain configuration")); + virReportError(VIR_ERR_OPERATION_INVALID, + _("memory device of model '%s' not found " + "in domain configuration"), + virDomainMemoryModelTypeToString(memdef->model));

model '%s' memory device not present in the domain configuration

return -1; }

John

$SUBJ: There should be two patches... One just for the error message (see commit id 'f0e7f90bff' for the last change to add a new error)... Thus commit message 1 becomes: qemu: Introduce VIR_ERR_DEVICE_MISSING and commit message 2 becomes: qemu: Use VIR_ERR_DEVICE_MISSING for various hotplug messages On 01/05/2018 05:28 AM, Chen Hanxiao wrote:

From: Chen Hanxiao <chenhanxiao@gmail.com>

We used VIR_ERR_OPERATION_FAILED when target detaching device is not found. That error code VIR_ERR_OPERATION_FAILED is widely used, so the tools powered by libvirt, such as nova, can't catch the exact errors from libvirt. This patch introduce VIR_ERR_DEVICE_MISSING for this kind of scenario.

The line breaks are "unusual" Commit message 1: "Add new error code to be able to allow consumers (such as Nova) to be able to key of a specific error code rather than needing to search the error message." Commit message 2: "Modify OPERATION_FAILED error codes to use DEVICE_MISSING instead."

Signed-off-by: Chen Hanxiao <chenhanxiao@gmail.com> --- include/libvirt/virterror.h | 1 + src/qemu/qemu_hotplug.c | 26 +++++++++++++------------- src/util/virerror.c | 6 ++++++

Don't forget virDomainNetFindIdx You may also want to search the sources for "device not found" type messages and have them use the new code as well. Of course they'd have some redundancy in the messages... No need to rush as 4.0.0 deadline is missed, but 4.1.0 should be fine. John

3 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h index 91ba29784..3e7c7a02c 100644 --- a/include/libvirt/virterror.h +++ b/include/libvirt/virterror.h @@ -320,6 +320,7 @@ typedef enum { VIR_ERR_AGENT_UNSYNCED = 97, /* guest agent replies with wrong id to guest-sync command (DEPRECATED)*/ VIR_ERR_LIBSSH = 98, /* error in libssh transport driver */ + VIR_ERR_DEVICE_MISSING = 99, /* fail to find the desired device */ } virErrorNumber;

/** diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 6472a13a8..9e4424e35 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -3513,7 +3513,7 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver, int ret = -1;

if (!olddev) { - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_DEVICE_MISSING, _("cannot find existing graphics device to modify of" " type '%s'"), type); goto cleanup; @@ -4758,7 +4758,7 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver, if ((idx = virDomainControllerFind(vm->def, dev->data.controller->type, dev->data.controller->idx)) < 0) { - virReportError(VIR_ERR_OPERATION_FAILED, + virReportError(VIR_ERR_DEVICE_MISSING, _("controller %s:%d not found"), virDomainControllerTypeToString(dev->data.controller->type), dev->data.controller->idx); @@ -4987,18 +4987,18 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, if (idx < 0) { switch (subsys->type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: - virReportError(VIR_ERR_OPERATION_FAILED, + virReportError(VIR_ERR_DEVICE_MISSING, _("host pci device %.4x:%.2x:%.2x.%.1x not found"), pcisrc->addr.domain, pcisrc->addr.bus, pcisrc->addr.slot, pcisrc->addr.function); break; case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: if (usbsrc->bus && usbsrc->device) { - virReportError(VIR_ERR_OPERATION_FAILED, + virReportError(VIR_ERR_DEVICE_MISSING, _("host usb device %03d.%03d not found"), usbsrc->bus, usbsrc->device); } else { - virReportError(VIR_ERR_OPERATION_FAILED, + virReportError(VIR_ERR_DEVICE_MISSING, _("host usb device vendor=0x%.4x product=0x%.4x not found"), usbsrc->vendor, usbsrc->product); } @@ -5007,13 +5007,13 @@ int qemuDomainDetachHostDevice(virQEMUDriverPtr driver, if (scsisrc->protocol == VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI) { virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi; - virReportError(VIR_ERR_OPERATION_FAILED, + virReportError(VIR_ERR_DEVICE_MISSING, _("host scsi iSCSI path %s not found"), iscsisrc->src->path); } else { virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host; - virReportError(VIR_ERR_OPERATION_FAILED, + virReportError(VIR_ERR_DEVICE_MISSING, _("host scsi device %s:%u:%u.%llu not found"), scsihostsrc->adapter, scsihostsrc->bus, scsihostsrc->target, scsihostsrc->unit); @@ -5051,7 +5051,7 @@ qemuDomainDetachShmemDevice(virQEMUDriverPtr driver, qemuDomainObjPrivatePtr priv = vm->privateData;

if ((idx = virDomainShmemDefFind(vm->def, dev)) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, + virReportError(VIR_ERR_DEVICE_MISSING, _("shmem device of model '%s' not found " "in domain configuration"), virDomainShmemModelTypeToString(dev->model)); @@ -5110,7 +5110,7 @@ qemuDomainDetachWatchdog(virQEMUDriverPtr driver, watchdog->model == dev->model && watchdog->action == dev->action && virDomainDeviceInfoAddressIsEqual(&dev->info, &watchdog->info))) { - virReportError(VIR_ERR_OPERATION_INVALID, + virReportError(VIR_ERR_DEVICE_MISSING, _("watchdog device of model '%s' is not " "found in domain configuration"), virDomainWatchdogModelTypeToString(watchdog->model)); @@ -5155,7 +5155,7 @@ qemuDomainDetachNetDevice(virQEMUDriverPtr driver,

if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0) { char mac[VIR_MAC_STRING_BUFLEN]; - virReportError(VIR_ERR_INTERNAL_ERROR, + virReportError(VIR_ERR_DEVICE_MISSING, _("netdev '%s' not found in domain configuration"), virMacAddrFormat(&dev->data.net->mac, mac)); goto cleanup; @@ -5345,7 +5345,7 @@ int qemuDomainDetachChrDevice(virQEMUDriverPtr driver, char *devstr = NULL;

if (!(tmpChr = virDomainChrFind(vmdef, chr))) { - virReportError(VIR_ERR_OPERATION_INVALID, + virReportError(VIR_ERR_DEVICE_MISSING, _("chr device of type '%s' not found " "in domain configuration"), virDomainChrDeviceTypeToString(chr->deviceType)); @@ -5394,7 +5394,7 @@ qemuDomainDetachRNGDevice(virQEMUDriverPtr driver, int ret = -1;

if ((idx = virDomainRNGFind(vm->def, rng)) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, + virReportError(VIR_ERR_DEVICE_MISSING, _("RNG device of model '%s' not found " "in domain configuration"), virDomainRNGBackendTypeToString(rng->model)); @@ -5439,7 +5439,7 @@ qemuDomainDetachMemoryDevice(virQEMUDriverPtr driver, qemuDomainMemoryDeviceAlignSize(vm->def, memdef);

if ((idx = virDomainMemoryFindByDef(vm->def, memdef)) < 0) { - virReportError(VIR_ERR_OPERATION_INVALID, + virReportError(VIR_ERR_DEVICE_MISSING, _("memory device of model '%s' not found " "in domain configuration"), virDomainMemoryModelTypeToString(memdef->model)); diff --git a/src/util/virerror.c b/src/util/virerror.c index 562c3bc61..c000b0043 100644 --- a/src/util/virerror.c +++ b/src/util/virerror.c @@ -1453,6 +1453,12 @@ virErrorMsg(virErrorNumber error, const char *info) else errmsg = _("libssh transport error: %s"); break; + case VIR_ERR_DEVICE_MISSING: + if (info == NULL) + errmsg = _("device not found"); + else + errmsg = _("device not found: %s"); + break; } return errmsg; }