subject:"\[libvirt\] \[PATCH V2\] libxl\: add default controllers for USB devices"

Re: [libvirt] [PATCH V2] libxl: add default controllers for USB devices

2017-05-23 Thread Jim Fehlig


On 05/23/2017 01:11 AM, Cedric Bosdonnat wrote:

On Fri, 2017-05-05 at 15:48 -0600, Jim Fehlig wrote:

Attempting to start a domain with USB hostdevs but no USB controllers
fails with the rather cryptic error

libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
error message from QMP server: Bus 'xenusb-0.0' not found

This can be fixed by creating default USB controllers. When no USB
controllers are defined, create the number of 8 port controllers
necessary to accommodate the number of defined USB devices.

Note that USB controllers are already created as needed in the
domainAttachDevice code path. E.g. a USB controller will be created,
if necessary, when attaching a USB device with
'virsh attach-device dom usbdev.xml'.

Signed-off-by: Jim Fehlig 
---

V1 here

https://www.redhat.com/archives/libvir-list/2017-April/msg00965.html

While further testing of V1 found a libvirtd segfault due to
incorrectly using virDomainControllerInsertPreAlloced instead of
virDomainControllerInsert.

  src/libxl/libxl_conf.c | 82 +++---
  1 file changed, 71 insertions(+), 11 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 56bc09719..cdf6ec9f3 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1822,34 +1822,94 @@ libxlMakeUSBController(virDomainControllerDefPtr 
controller,
  }
  
  static int

+libxlMakeDefaultUSBControllers(virDomainDefPtr def,
+   libxl_domain_config *d_config)
+{
+virDomainControllerDefPtr l_controller = NULL;
+libxl_device_usbctrl *x_controllers = NULL;
+size_t nusbdevs = 0;
+size_t ncontrollers;
+size_t i;
+
+for (i = 0; i < def->nhostdevs; i++) {
+if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+def->hostdevs[i]->source.subsys.type == 
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+nusbdevs++;
+}
+
+/* No controllers needed if there are no USB devs */
+if (nusbdevs == 0)
+return 0;
+
+/* Create USB controllers with 8 ports */
+ncontrollers = VIR_DIV_UP(nusbdevs, 8);
+if (VIR_ALLOC_N(x_controllers, ncontrollers) < 0)
+return -1;
+
+for (i = 0; i < ncontrollers; i++) {
+if (!(l_controller = 
virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_USB)))
+goto error;
+
+l_controller->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2;
+l_controller->idx = i;
+l_controller->opts.usbopts.ports = 8;
+
+libxl_device_usbctrl_init(_controllers[i]);
+
+if (libxlMakeUSBController(l_controller, _controllers[i]) < 0)
+goto error;
+
+if (virDomainControllerInsert(def, l_controller) < 0)
+goto error;
+
+l_controller = NULL;
+}
+
+d_config->usbctrls = x_controllers;
+d_config->num_usbctrls = ncontrollers;
+return 0;
+
+ error:
+ virDomainControllerDefFree(l_controller);
+ for (i = 0; i < ncontrollers; i++)
+ libxl_device_usbctrl_dispose(_controllers[i]);
+ VIR_FREE(x_controllers);
+ return -1;
+}
+
+static int
  libxlMakeUSBControllerList(virDomainDefPtr def, libxl_domain_config *d_config)
  {
  virDomainControllerDefPtr *l_controllers = def->controllers;
  size_t ncontrollers = def->ncontrollers;
  size_t nusbctrls = 0;
  libxl_device_usbctrl *x_usbctrls;
-size_t i;
-
-if (ncontrollers == 0)
-return 0;
-
-if (VIR_ALLOC_N(x_usbctrls, ncontrollers) < 0)
-return -1;
+size_t i, j;
  
  for (i = 0; i < ncontrollers; i++) {

+if (l_controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB)
+nusbctrls++;
+}
+
+if (nusbctrls == 0)
+return libxlMakeDefaultUSBControllers(def, d_config);
+
+if (VIR_ALLOC_N(x_usbctrls, nusbctrls) < 0)
+return -1;
+
+for (i = 0, j = 0; i < ncontrollers && j < nusbctrls; i++) {
  if (l_controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
  continue;
  
-libxl_device_usbctrl_init(_usbctrls[nusbctrls]);

+libxl_device_usbctrl_init(_usbctrls[j]);
  
  if (libxlMakeUSBController(l_controllers[i],

-   _usbctrls[nusbctrls]) < 0)
+   _usbctrls[j]) < 0)
  goto error;
  
-nusbctrls++;

+j++;
  }
  
-VIR_SHRINK_N(x_usbctrls, ncontrollers, ncontrollers - nusbctrls);

  d_config->usbctrls = x_usbctrls;
  d_config->num_usbctrls = nusbctrls;
  


ACK


Cool, thanks for taking a look! Pushed now.

Regards,
Jim

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

Re: [libvirt] [PATCH V2] libxl: add default controllers for USB devices

2017-05-23 Thread Cedric Bosdonnat

On Fri, 2017-05-05 at 15:48 -0600, Jim Fehlig wrote:
> Attempting to start a domain with USB hostdevs but no USB controllers
> fails with the rather cryptic error
> 
> libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
> error message from QMP server: Bus 'xenusb-0.0' not found
> 
> This can be fixed by creating default USB controllers. When no USB
> controllers are defined, create the number of 8 port controllers
> necessary to accommodate the number of defined USB devices.
> 
> Note that USB controllers are already created as needed in the
> domainAttachDevice code path. E.g. a USB controller will be created,
> if necessary, when attaching a USB device with
> 'virsh attach-device dom usbdev.xml'.
> 
> Signed-off-by: Jim Fehlig 
> ---
> 
> V1 here
> 
> https://www.redhat.com/archives/libvir-list/2017-April/msg00965.html
> 
> While further testing of V1 found a libvirtd segfault due to
> incorrectly using virDomainControllerInsertPreAlloced instead of
> virDomainControllerInsert.
> 
>  src/libxl/libxl_conf.c | 82 
> +++---
>  1 file changed, 71 insertions(+), 11 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 56bc09719..cdf6ec9f3 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1822,34 +1822,94 @@ libxlMakeUSBController(virDomainControllerDefPtr 
> controller,
>  }
>  
>  static int
> +libxlMakeDefaultUSBControllers(virDomainDefPtr def,
> +   libxl_domain_config *d_config)
> +{
> +virDomainControllerDefPtr l_controller = NULL;
> +libxl_device_usbctrl *x_controllers = NULL;
> +size_t nusbdevs = 0;
> +size_t ncontrollers;
> +size_t i;
> +
> +for (i = 0; i < def->nhostdevs; i++) {
> +if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
> +def->hostdevs[i]->source.subsys.type == 
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
> +nusbdevs++;
> +}
> +
> +/* No controllers needed if there are no USB devs */
> +if (nusbdevs == 0)
> +return 0;
> +
> +/* Create USB controllers with 8 ports */
> +ncontrollers = VIR_DIV_UP(nusbdevs, 8);
> +if (VIR_ALLOC_N(x_controllers, ncontrollers) < 0)
> +return -1;
> +
> +for (i = 0; i < ncontrollers; i++) {
> +if (!(l_controller = 
> virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_USB)))
> +goto error;
> +
> +l_controller->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2;
> +l_controller->idx = i;
> +l_controller->opts.usbopts.ports = 8;
> +
> +libxl_device_usbctrl_init(_controllers[i]);
> +
> +if (libxlMakeUSBController(l_controller, _controllers[i]) < 0)
> +goto error;
> +
> +if (virDomainControllerInsert(def, l_controller) < 0)
> +goto error;
> +
> +l_controller = NULL;
> +}
> +
> +d_config->usbctrls = x_controllers;
> +d_config->num_usbctrls = ncontrollers;
> +return 0;
> +
> + error:
> + virDomainControllerDefFree(l_controller);
> + for (i = 0; i < ncontrollers; i++)
> + libxl_device_usbctrl_dispose(_controllers[i]);
> + VIR_FREE(x_controllers);
> + return -1;
> +}
> +
> +static int
>  libxlMakeUSBControllerList(virDomainDefPtr def, libxl_domain_config 
> *d_config)
>  {
>  virDomainControllerDefPtr *l_controllers = def->controllers;
>  size_t ncontrollers = def->ncontrollers;
>  size_t nusbctrls = 0;
>  libxl_device_usbctrl *x_usbctrls;
> -size_t i;
> -
> -if (ncontrollers == 0)
> -return 0;
> -
> -if (VIR_ALLOC_N(x_usbctrls, ncontrollers) < 0)
> -return -1;
> +size_t i, j;
>  
>  for (i = 0; i < ncontrollers; i++) {
> +if (l_controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB)
> +nusbctrls++;
> +}
> +
> +if (nusbctrls == 0)
> +return libxlMakeDefaultUSBControllers(def, d_config);
> +
> +if (VIR_ALLOC_N(x_usbctrls, nusbctrls) < 0)
> +return -1;
> +
> +for (i = 0, j = 0; i < ncontrollers && j < nusbctrls; i++) {
>  if (l_controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
>  continue;
>  
> -libxl_device_usbctrl_init(_usbctrls[nusbctrls]);
> +libxl_device_usbctrl_init(_usbctrls[j]);
>  
>  if (libxlMakeUSBController(l_controllers[i],
> -   _usbctrls[nusbctrls]) < 0)
> +   _usbctrls[j]) < 0)
>  goto error;
>  
> -nusbctrls++;
> +j++;
>  }
>  
> -VIR_SHRINK_N(x_usbctrls, ncontrollers, ncontrollers - nusbctrls);
>  d_config->usbctrls = x_usbctrls;
>  d_config->num_usbctrls = nusbctrls;
>  

ACK

--
Cedric

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

Re: [libvirt] [PATCH V2] libxl: add default controllers for USB devices

2017-05-16 Thread Jim Fehlig

Anyone have a few spare minutes to review this patch? TIA!

Regards,
Jim

Jim Fehlig wrote:
> Attempting to start a domain with USB hostdevs but no USB controllers
> fails with the rather cryptic error
> 
> libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
> error message from QMP server: Bus 'xenusb-0.0' not found
> 
> This can be fixed by creating default USB controllers. When no USB
> controllers are defined, create the number of 8 port controllers
> necessary to accommodate the number of defined USB devices.
> 
> Note that USB controllers are already created as needed in the
> domainAttachDevice code path. E.g. a USB controller will be created,
> if necessary, when attaching a USB device with
> 'virsh attach-device dom usbdev.xml'.
> 
> Signed-off-by: Jim Fehlig 
> ---
> 
> V1 here
> 
> https://www.redhat.com/archives/libvir-list/2017-April/msg00965.html
> 
> While further testing of V1 found a libvirtd segfault due to
> incorrectly using virDomainControllerInsertPreAlloced instead of
> virDomainControllerInsert.
> 
>  src/libxl/libxl_conf.c | 82 
> +++---
>  1 file changed, 71 insertions(+), 11 deletions(-)
> 
> diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
> index 56bc09719..cdf6ec9f3 100644
> --- a/src/libxl/libxl_conf.c
> +++ b/src/libxl/libxl_conf.c
> @@ -1822,34 +1822,94 @@ libxlMakeUSBController(virDomainControllerDefPtr 
> controller,
>  }
>  
>  static int
> +libxlMakeDefaultUSBControllers(virDomainDefPtr def,
> +   libxl_domain_config *d_config)
> +{
> +virDomainControllerDefPtr l_controller = NULL;
> +libxl_device_usbctrl *x_controllers = NULL;
> +size_t nusbdevs = 0;
> +size_t ncontrollers;
> +size_t i;
> +
> +for (i = 0; i < def->nhostdevs; i++) {
> +if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
> +def->hostdevs[i]->source.subsys.type == 
> VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
> +nusbdevs++;
> +}
> +
> +/* No controllers needed if there are no USB devs */
> +if (nusbdevs == 0)
> +return 0;
> +
> +/* Create USB controllers with 8 ports */
> +ncontrollers = VIR_DIV_UP(nusbdevs, 8);
> +if (VIR_ALLOC_N(x_controllers, ncontrollers) < 0)
> +return -1;
> +
> +for (i = 0; i < ncontrollers; i++) {
> +if (!(l_controller = 
> virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_USB)))
> +goto error;
> +
> +l_controller->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2;
> +l_controller->idx = i;
> +l_controller->opts.usbopts.ports = 8;
> +
> +libxl_device_usbctrl_init(_controllers[i]);
> +
> +if (libxlMakeUSBController(l_controller, _controllers[i]) < 0)
> +goto error;
> +
> +if (virDomainControllerInsert(def, l_controller) < 0)
> +goto error;
> +
> +l_controller = NULL;
> +}
> +
> +d_config->usbctrls = x_controllers;
> +d_config->num_usbctrls = ncontrollers;
> +return 0;
> +
> + error:
> + virDomainControllerDefFree(l_controller);
> + for (i = 0; i < ncontrollers; i++)
> + libxl_device_usbctrl_dispose(_controllers[i]);
> + VIR_FREE(x_controllers);
> + return -1;
> +}
> +
> +static int
>  libxlMakeUSBControllerList(virDomainDefPtr def, libxl_domain_config 
> *d_config)
>  {
>  virDomainControllerDefPtr *l_controllers = def->controllers;
>  size_t ncontrollers = def->ncontrollers;
>  size_t nusbctrls = 0;
>  libxl_device_usbctrl *x_usbctrls;
> -size_t i;
> -
> -if (ncontrollers == 0)
> -return 0;
> -
> -if (VIR_ALLOC_N(x_usbctrls, ncontrollers) < 0)
> -return -1;
> +size_t i, j;
>  
>  for (i = 0; i < ncontrollers; i++) {
> +if (l_controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB)
> +nusbctrls++;
> +}
> +
> +if (nusbctrls == 0)
> +return libxlMakeDefaultUSBControllers(def, d_config);
> +
> +if (VIR_ALLOC_N(x_usbctrls, nusbctrls) < 0)
> +return -1;
> +
> +for (i = 0, j = 0; i < ncontrollers && j < nusbctrls; i++) {
>  if (l_controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
>  continue;
>  
> -libxl_device_usbctrl_init(_usbctrls[nusbctrls]);
> +libxl_device_usbctrl_init(_usbctrls[j]);
>  
>  if (libxlMakeUSBController(l_controllers[i],
> -   _usbctrls[nusbctrls]) < 0)
> +   _usbctrls[j]) < 0)
>  goto error;
>  
> -nusbctrls++;
> +j++;
>  }
>  
> -VIR_SHRINK_N(x_usbctrls, ncontrollers, ncontrollers - nusbctrls);
>  d_config->usbctrls = x_usbctrls;
>  d_config->num_usbctrls = nusbctrls;
>  

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

[libvirt] [PATCH V2] libxl: add default controllers for USB devices

2017-05-05 Thread Jim Fehlig

Attempting to start a domain with USB hostdevs but no USB controllers
fails with the rather cryptic error

libxl: error: libxl_qmp.c:287:qmp_handle_error_response: received an
error message from QMP server: Bus 'xenusb-0.0' not found

This can be fixed by creating default USB controllers. When no USB
controllers are defined, create the number of 8 port controllers
necessary to accommodate the number of defined USB devices.

Note that USB controllers are already created as needed in the
domainAttachDevice code path. E.g. a USB controller will be created,
if necessary, when attaching a USB device with
'virsh attach-device dom usbdev.xml'.

Signed-off-by: Jim Fehlig 
---

V1 here

https://www.redhat.com/archives/libvir-list/2017-April/msg00965.html

While further testing of V1 found a libvirtd segfault due to
incorrectly using virDomainControllerInsertPreAlloced instead of
virDomainControllerInsert.

 src/libxl/libxl_conf.c | 82 +++---
 1 file changed, 71 insertions(+), 11 deletions(-)

diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index 56bc09719..cdf6ec9f3 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1822,34 +1822,94 @@ libxlMakeUSBController(virDomainControllerDefPtr 
controller,
 }
 
 static int
+libxlMakeDefaultUSBControllers(virDomainDefPtr def,
+   libxl_domain_config *d_config)
+{
+virDomainControllerDefPtr l_controller = NULL;
+libxl_device_usbctrl *x_controllers = NULL;
+size_t nusbdevs = 0;
+size_t ncontrollers;
+size_t i;
+
+for (i = 0; i < def->nhostdevs; i++) {
+if (def->hostdevs[i]->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS &&
+def->hostdevs[i]->source.subsys.type == 
VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB)
+nusbdevs++;
+}
+
+/* No controllers needed if there are no USB devs */
+if (nusbdevs == 0)
+return 0;
+
+/* Create USB controllers with 8 ports */
+ncontrollers = VIR_DIV_UP(nusbdevs, 8);
+if (VIR_ALLOC_N(x_controllers, ncontrollers) < 0)
+return -1;
+
+for (i = 0; i < ncontrollers; i++) {
+if (!(l_controller = 
virDomainControllerDefNew(VIR_DOMAIN_CONTROLLER_TYPE_USB)))
+goto error;
+
+l_controller->model = VIR_DOMAIN_CONTROLLER_MODEL_USB_QUSB2;
+l_controller->idx = i;
+l_controller->opts.usbopts.ports = 8;
+
+libxl_device_usbctrl_init(_controllers[i]);
+
+if (libxlMakeUSBController(l_controller, _controllers[i]) < 0)
+goto error;
+
+if (virDomainControllerInsert(def, l_controller) < 0)
+goto error;
+
+l_controller = NULL;
+}
+
+d_config->usbctrls = x_controllers;
+d_config->num_usbctrls = ncontrollers;
+return 0;
+
+ error:
+ virDomainControllerDefFree(l_controller);
+ for (i = 0; i < ncontrollers; i++)
+ libxl_device_usbctrl_dispose(_controllers[i]);
+ VIR_FREE(x_controllers);
+ return -1;
+}
+
+static int
 libxlMakeUSBControllerList(virDomainDefPtr def, libxl_domain_config *d_config)
 {
 virDomainControllerDefPtr *l_controllers = def->controllers;
 size_t ncontrollers = def->ncontrollers;
 size_t nusbctrls = 0;
 libxl_device_usbctrl *x_usbctrls;
-size_t i;
-
-if (ncontrollers == 0)
-return 0;
-
-if (VIR_ALLOC_N(x_usbctrls, ncontrollers) < 0)
-return -1;
+size_t i, j;
 
 for (i = 0; i < ncontrollers; i++) {
+if (l_controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_USB)
+nusbctrls++;
+}
+
+if (nusbctrls == 0)
+return libxlMakeDefaultUSBControllers(def, d_config);
+
+if (VIR_ALLOC_N(x_usbctrls, nusbctrls) < 0)
+return -1;
+
+for (i = 0, j = 0; i < ncontrollers && j < nusbctrls; i++) {
 if (l_controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_USB)
 continue;
 
-libxl_device_usbctrl_init(_usbctrls[nusbctrls]);
+libxl_device_usbctrl_init(_usbctrls[j]);
 
 if (libxlMakeUSBController(l_controllers[i],
-   _usbctrls[nusbctrls]) < 0)
+   _usbctrls[j]) < 0)
 goto error;
 
-nusbctrls++;
+j++;
 }
 
-VIR_SHRINK_N(x_usbctrls, ncontrollers, ncontrollers - nusbctrls);
 d_config->usbctrls = x_usbctrls;
 d_config->num_usbctrls = nusbctrls;
 
-- 
2.11.0

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

Re: [libvirt] [PATCH V2] libxl: add default controllers for USB devices

Re: [libvirt] [PATCH V2] libxl: add default controllers for USB devices

Re: [libvirt] [PATCH V2] libxl: add default controllers for USB devices

[libvirt] [PATCH V2] libxl: add default controllers for USB devices

4 matches

Site Navigation

Mail list logo

Footer information