[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [v11 4/6] change lxc driver to use hostdev common library



Chunyan Liu wrote:
> Change lxc driver to use hostdev common library instead of APIs in
> lxc_hostdev.[ch]
>
> Signed-off-by: Chunyan Liu <cyliu suse com>
> ---
>  po/POTFILES.in        |    1 -
>  src/Makefile.am       |    1 -
>  src/lxc/lxc_conf.h    |    4 -
>  src/lxc/lxc_driver.c  |   47 ++++---
>  src/lxc/lxc_hostdev.c |  416 -------------------------------------------------
>  src/lxc/lxc_hostdev.h |   43 -----
>  src/lxc/lxc_process.c |   24 +++-
>  7 files changed, 48 insertions(+), 488 deletions(-)
>  delete mode 100644 src/lxc/lxc_hostdev.c
>  delete mode 100644 src/lxc/lxc_hostdev.h
>
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 9e71db3..94017c6 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -61,7 +61,6 @@ src/locking/lock_manager.c
>  src/locking/sanlock_helper.c
>  src/lxc/lxc_cgroup.c
>  src/lxc/lxc_fuse.c
> -src/lxc/lxc_hostdev.c
>  src/lxc/lxc_container.c
>  src/lxc/lxc_conf.c
>  src/lxc/lxc_controller.c
> diff --git a/src/Makefile.am b/src/Makefile.am
> index e8b6fc4..20ceec2 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -619,7 +619,6 @@ LXC_DRIVER_SOURCES =						\
>  		lxc/lxc_container.c lxc/lxc_container.h		\
>  		lxc/lxc_cgroup.c lxc/lxc_cgroup.h		\
>  		lxc/lxc_domain.c lxc/lxc_domain.h		\
> -		lxc/lxc_hostdev.c lxc/lxc_hostdev.h		\
>  		lxc/lxc_monitor.c lxc/lxc_monitor.h		\
>  		lxc/lxc_process.c lxc/lxc_process.h		\
>  		lxc/lxc_fuse.c lxc/lxc_fuse.h			\
> diff --git a/src/lxc/lxc_conf.h b/src/lxc/lxc_conf.h
> index e04dcdd..5be159b 100644
> --- a/src/lxc/lxc_conf.h
> +++ b/src/lxc/lxc_conf.h
> @@ -93,10 +93,6 @@ struct _virLXCDriver {
>      /* Immutable pointer, self-locking APIs */
>      virDomainObjListPtr domains;
>  
> -    /* Immutable pointer. Requires lock to be held before
> -     * calling APIs. */
> -    virUSBDeviceListPtr activeUsbHostdevs;
> -
>      /* Immutable pointer, self-locking APIs */
>      virObjectEventStatePtr domainEventState;
>  
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index 982f3fc..3a62638 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -70,6 +70,7 @@
>  #include "virstring.h"
>  #include "viraccessapicheck.h"
>  #include "viraccessapichecklxc.h"
> +#include "virhostdev.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_LXC
>  
> @@ -1526,9 +1527,6 @@ static int lxcStateInitialize(bool privileged,
>      if (!(lxc_driver->securityManager = lxcSecurityInit(cfg)))
>          goto cleanup;
>  
> -    if ((lxc_driver->activeUsbHostdevs = virUSBDeviceListNew()) == NULL)
> -        goto cleanup;
> -
>      if ((virLXCDriverGetCapabilities(lxc_driver, true)) == NULL)
>          goto cleanup;
>  
> @@ -1643,7 +1641,6 @@ static int lxcStateCleanup(void)
>  
>      virSysinfoDefFree(lxc_driver->hostsysinfo);
>  
> -    virObjectUnref(lxc_driver->activeUsbHostdevs);
>      virObjectUnref(lxc_driver->caps);
>      virObjectUnref(lxc_driver->securityManager);
>      virObjectUnref(lxc_driver->xmlopt);
> @@ -3903,6 +3900,7 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver,
>      mode_t mode;
>      bool created = false;
>      virUSBDevicePtr usb = NULL;
> +    virHostdevManagerPtr hostdev_mgr;
>  
>      if (virDomainHostdevFind(vm->def, def, NULL) >= 0) {
>          virReportError(VIR_ERR_OPERATION_FAILED, "%s",
> @@ -3910,6 +3908,14 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver,
>          return -1;
>      }
>  
> +    hostdev_mgr = virHostdevManagerGetDefault();
> +    if (hostdev_mgr == NULL ||
> +        virHostdevPrepareUsbHostdevs(hostdev_mgr,
> +                                     LXC_DRIVER_NAME,
> +                                     vm->def->name,
> +                                     &def, 1, 0) < 0)
> +        return -1;
>   

Should an error be reported here?

> +
>      if (virAsprintf(&vroot, "/proc/%llu/root",
>                      (unsigned long long)priv->initpid) < 0)
>          goto cleanup;
> @@ -3985,6 +3991,11 @@ lxcDomainAttachDeviceHostdevSubsysUSBLive(virLXCDriverPtr driver,
>      ret = 0;
>  
>  cleanup:
> +    virHostdevReAttachUsbHostdevs(hostdev_mgr,
> +                                  LXC_DRIVER_NAME,
> +                                  vm->def->name,
> +                                  &def,
> +                                  1);
>      virDomainAuditHostdev(vm, def, "attach", ret == 0);
>      if (ret < 0 && created)
>          unlink(dstfile);
> @@ -4447,8 +4458,7 @@ cleanup:
>  
>  
>  static int
> -lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver,
> -                                    virDomainObjPtr vm,
> +lxcDomainDetachDeviceHostdevUSBLive(virDomainObjPtr vm,
>                                      virDomainDeviceDefPtr dev)
>  {
>      virLXCDomainObjPrivatePtr priv = vm->privateData;
> @@ -4457,6 +4467,7 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver,
>      char *dst = NULL;
>      char *vroot = NULL;
>      virUSBDevicePtr usb = NULL;
> +    virHostdevManagerPtr hostdev_mgr;
>  
>      if ((idx = virDomainHostdevFind(vm->def,
>                                      dev->data.hostdev,
> @@ -4501,9 +4512,10 @@ lxcDomainDetachDeviceHostdevUSBLive(virLXCDriverPtr driver,
>          VIR_WARN("cannot deny device %s for domain %s",
>                   dst, vm->def->name);
>  
> -    virObjectLock(driver->activeUsbHostdevs);
> -    virUSBDeviceListDel(driver->activeUsbHostdevs, usb);
> -    virObjectUnlock(driver->activeUsbHostdevs);
> +    hostdev_mgr = virHostdevManagerGetDefault();
> +    if (hostdev_mgr != NULL)
> +        virHostdevReAttachPciHostdevs(hostdev_mgr, LXC_DRIVER_NAME,
> +                                      vm->def->name, &def, 1);
>   

Here, and elsewhere in this patch, it seems prudent to report an error
and fail if hostdev_mgr is NULL.

Chunyan:  I poked Cedric about reviewing this patch and testing the
series on LXC.  While rebasing, he noticed some discrepancies relative
to the latest code (probably from the many previous rebases), and kindly
"volunteered" to help move the series along while you're on
vacation/holiday.  So don't be surprised to see a V12 when you return
:-).  Thanks for your patience!

Laine: Thanks for nudge regarding a closer look at the series.  Lesson
learned about multiple rebases of a non-trivial patchset against a very
active code base :-).

Regards,
Jim


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]