[libvirt] [PATCH v2 1/2] qemu_hotplug: more proper error messages when target detaching device is not found

[John Ferlan]

$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 at gmail.com>
> 
> More proper/detail error messages updated.>
> Signed-off-by: Chen Hanxiao <chenhanxiao at 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