Re: [Xen-devel] [PATCH 2/3] libxl: attach PCI device to qemu only after setting pciback/pcifront

2016-10-19 Thread Wei Liu
On Tue, Oct 18, 2016 at 03:53:32AM +0200, Marek Marczykowski-Górecki wrote:
> When qemu is running in stubdomain, handling "pci-ins" command will fail
> if pcifront is not initialized already. Fix this by sending such command
> only after confirming that pciback/front is running.
> 
> It is possible to handle this case in mini-os code, by delaying
> "pci-ins" handling until pcifront_thread finishes devices discovery. But
> the same problem most likely will apply to any other stubdomain
> implementations when they come (Rumprun, Linux) - so better handle this
> at libxl level.
> 
> Signed-off-by: Marek Marczykowski-Górecki 

Mostly some coding style issues.

> ---
>  tools/libxl/libxl_pci.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
> index 2ae1bc4..3805d30 100644
> --- a/tools/libxl/libxl_pci.c
> +++ b/tools/libxl/libxl_pci.c
> @@ -1195,6 +1195,7 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t 
> domid, libxl_device_pci *pcide
>  {
>  libxl_ctx *ctx = libxl__gc_owner(gc);
>  unsigned int orig_vdev, pfunc_mask;
> +char *be_path;
>  libxl_device_pci *assigned;
>  int num_assigned, i, rc;
>  int stubdomid = 0;
> @@ -1249,6 +1250,14 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t 
> domid, libxl_device_pci *pcide
>  rc = do_pci_add(gc, stubdomid, _s, 0);
>  if ( rc )
>  goto out;
> +/* wait for the device actually being connected, otherwise device 
> model
> + * running there will fail to find the device */

Hmm... I don't think we have written down style requirement for code
comment, but I would like this comment to be formatted a bit.

/* Wait for the device actually being connected, otherwise device model
 * running there will fail to find the device. */

That is to capitalise the first word and make it a proper sentence.

> +be_path = libxl__sprintf(gc, "%s/backend/pci/%d/0",
> +libxl__xs_get_dompath(gc, 0), stubdomid);
> +rc = libxl__wait_for_backend(gc, be_path,
> +GCSPRINTF("%d", XenbusStateConnected));

Please indent this a bit.

> +if ( rc )

Not really your problem. This file is ridden with such style violation.
Can you please remove the space before and after "rc"?

Wei.

> +goto out;
>  }
>  
>  orig_vdev = pcidev->vdevfn & ~7U;
> -- 
> 2.5.5
> 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel


[Xen-devel] [PATCH 2/3] libxl: attach PCI device to qemu only after setting pciback/pcifront

2016-10-17 Thread Marek Marczykowski-Górecki
When qemu is running in stubdomain, handling "pci-ins" command will fail
if pcifront is not initialized already. Fix this by sending such command
only after confirming that pciback/front is running.

It is possible to handle this case in mini-os code, by delaying
"pci-ins" handling until pcifront_thread finishes devices discovery. But
the same problem most likely will apply to any other stubdomain
implementations when they come (Rumprun, Linux) - so better handle this
at libxl level.

Signed-off-by: Marek Marczykowski-Górecki 
---
 tools/libxl/libxl_pci.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c
index 2ae1bc4..3805d30 100644
--- a/tools/libxl/libxl_pci.c
+++ b/tools/libxl/libxl_pci.c
@@ -1195,6 +1195,7 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, 
libxl_device_pci *pcide
 {
 libxl_ctx *ctx = libxl__gc_owner(gc);
 unsigned int orig_vdev, pfunc_mask;
+char *be_path;
 libxl_device_pci *assigned;
 int num_assigned, i, rc;
 int stubdomid = 0;
@@ -1249,6 +1250,14 @@ int libxl__device_pci_add(libxl__gc *gc, uint32_t domid, 
libxl_device_pci *pcide
 rc = do_pci_add(gc, stubdomid, _s, 0);
 if ( rc )
 goto out;
+/* wait for the device actually being connected, otherwise device model
+ * running there will fail to find the device */
+be_path = libxl__sprintf(gc, "%s/backend/pci/%d/0",
+libxl__xs_get_dompath(gc, 0), stubdomid);
+rc = libxl__wait_for_backend(gc, be_path,
+GCSPRINTF("%d", XenbusStateConnected));
+if ( rc )
+goto out;
 }
 
 orig_vdev = pcidev->vdevfn & ~7U;
-- 
2.5.5


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel