Re: [PATCH v2 05/11] qemu: handle libvirtd restart after host usb device unplug

[Daniel Henrique Barboza <danielhb413@xxxxxxxxx>]

On 9/9/19 8:33 AM, Nikolay Shirokovskiy wrote:
> It is possible for libvirtd to go down before DEVICE_DELETED event is
> delivered upon usb hostdev unplug and to receive the event after the
> libvirtd is up. In order to handle this case we need to save
> usb hostdev deleteAction is status file.

nit: s/is/in ?


Reviewed-by: Daniel Henrique Barboza <danielhb413@xxxxxxxxx>



> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
> ---
>   src/conf/domain_conf.c | 26 ++++++++++++++++++++++++++
>   src/conf/domain_conf.h |  1 +
>   src/qemu/qemu_driver.c | 20 ++++++++++++++++++--
>   3 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b7a342bb91..c200af050c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1236,6 +1236,13 @@ VIR_ENUM_IMPL(virDomainShmemModel,
>                 "ivshmem-doorbell",
>   );
>   
> +VIR_ENUM_IMPL(virDomainHostdevDeleteAction,
> +              VIR_DOMAIN_HOSTDEV_DELETE_ACTION_LAST,
> +              "none",
> +              "delete",
> +              "unplug"
> +);
> +
>   VIR_ENUM_IMPL(virDomainLaunchSecurity,
>                 VIR_DOMAIN_LAUNCH_SECURITY_LAST,
>                 "",
> @@ -7533,6 +7540,7 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
>       virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb;
>       VIR_AUTOFREE(char *) startupPolicy = NULL;
>       VIR_AUTOFREE(char *) autoAddress = NULL;
> +    VIR_AUTOFREE(char *) deleteAction = NULL;
>   
>       if ((startupPolicy = virXMLPropString(node, "startupPolicy"))) {
>           def->startupPolicy =
> @@ -7550,6 +7558,18 @@ virDomainHostdevSubsysUSBDefParseXML(xmlNodePtr node,
>               usbsrc->autoAddress = true;
>       }
>   
> +    if ((deleteAction = virXMLPropString(node, "deleteAction"))) {
> +        def->deleteAction =
> +            virDomainHostdevDeleteActionTypeFromString(deleteAction);
> +
> +        if (def->deleteAction <= 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("Unknown deleteAction '%s'"),
> +                           deleteAction);
> +            goto out;
> +        }
> +    }
> +
>       /* Product can validly be 0, so we need some extra help to determine
>        * if it is uninitialized*/
>       got_product = false;
> @@ -24905,6 +24925,12 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
>   
>           if (def->missing && !(flags & VIR_DOMAIN_DEF_FORMAT_INACTIVE))
>               virBufferAddLit(buf, " missing='yes'");
> +
> +        if (def->deleteAction && (flags & VIR_DOMAIN_DEF_FORMAT_STATUS)) {
> +            const char *deleteAction;
> +            deleteAction = virDomainHostdevDeleteActionTypeToString(def->deleteAction);
> +            virBufferAsprintf(buf, " deleteAction='%s'", deleteAction);
> +        }
>       }
>   
>       if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 19a5b21462..df88790ac0 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -338,6 +338,7 @@ typedef enum {
>   
>       VIR_DOMAIN_HOSTDEV_DELETE_ACTION_LAST
>   } virDomainHostdevDeleteActionType;
> +VIR_ENUM_DECL(virDomainHostdevDeleteAction);
>   
>   
>   /* basic device for direct passthrough */
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index ce41b0a8d9..f1802b5d44 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -5272,10 +5272,13 @@ processUSBAddedEvent(virQEMUDriverPtr driver,
>   {
>       virDomainHostdevDefPtr hostdev;
>       virDomainHostdevSubsysUSBPtr usbsrc;
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>       size_t i;
>   
> -    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) {
> +        virObjectUnref(cfg);
>           return;
> +    }
>   
>       if (!virDomainObjIsActive(vm)) {
>           VIR_DEBUG("Domain is not running");
> @@ -5300,8 +5303,13 @@ processUSBAddedEvent(virQEMUDriverPtr driver,
>       if (qemuDomainAttachHostDevice(driver, vm, hostdev) < 0)
>           goto cleanup;
>   
> +    if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0)
> +        VIR_WARN("unable to save domain status after plugging device %s",
> +                 hostdev->info->alias);
> +
>    cleanup:
>       qemuDomainObjEndJob(driver, vm);
> +    virObjectUnref(cfg);
>   }
>   
>   
> @@ -5313,9 +5321,12 @@ processUSBRemovedEvent(virQEMUDriverPtr driver,
>       size_t i;
>       virDomainHostdevDefPtr hostdev;
>       virDomainDeviceDef dev = { .type = VIR_DOMAIN_DEVICE_HOSTDEV };
> +    virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
>   
> -    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> +    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) {
> +        virObjectUnref(cfg);
>           return;
> +    }
>   
>       if (!virDomainObjIsActive(vm)) {
>           VIR_DEBUG("Domain is not running");
> @@ -5348,8 +5359,13 @@ processUSBRemovedEvent(virQEMUDriverPtr driver,
>       if (qemuDomainDetachDeviceLive(vm, &dev, driver, true, true) < 0)
>           goto cleanup;
>   
> +    if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0)
> +        VIR_WARN("unable to save domain status after unplugging device %s",
> +                 hostdev->info->alias);
> +
>    cleanup:
>       qemuDomainObjEndJob(driver, vm);
> +    virObjectUnref(cfg);
>   }
>   
>   

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list