[PATCH] libxl: set channel devid when not provided by application

[PATCH] libxl: set channel devid when not provided by application

[Jim Fehlig]

Applications like libvirt may not populate a device devid field,
delegating that to libxl. If needed, the application can later
retrieve the libxl-produced devid. Indeed most devices are handled
this way in libvirt, channel devices included.

This works well when only one channel device is defined, but more
than one results in

qemu-system-i386: -chardev socket,id=libxl-channel-1,\
path=/tmp/test-org.qemu.guest_agent.00,server,nowait:
Duplicate ID 'libxl-channel-1' for chardev

Besides the odd '-1' value in the id, multiple channels have the same
id, causing qemu to fail. A simple fix is to set an uninitialized
devid (-1) to the dev_num passed to libxl__init_console_from_channel().

Signed-off-by: Jim Fehlig <jfehlig@suse.com>
---

I get the feeling that if needed devid should be set earlier, but
this seems like the most opportune spot. Suggestions for improvements
welcome.

 tools/libxl/libxl_console.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
index 39d8430df8..8faf3a24f3 100644
--- a/tools/libxl/libxl_console.c
+++ b/tools/libxl/libxl_console.c
@@ -401,6 +401,9 @@ int libxl__init_console_from_channel(libxl__gc *gc,
 
     /* Perform validation first, allocate second. */
 
+    if (channel->devid == -1)
+        channel->devid = dev_num;
+
     if (!channel->name) {
         LOG(ERROR, "channel %d has no name", channel->devid);
         return ERROR_INVAL;
-- 
2.16.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxl: set channel devid when not provided by application

[Jim Fehlig]

Any comments on this patch? Thanks!

Regards,
Jim

On 02/07/2018 08:04 PM, Jim Fehlig wrote:
> Applications like libvirt may not populate a device devid field,
> delegating that to libxl. If needed, the application can later
> retrieve the libxl-produced devid. Indeed most devices are handled
> this way in libvirt, channel devices included.
> 
> This works well when only one channel device is defined, but more
> than one results in
> 
> qemu-system-i386: -chardev socket,id=libxl-channel-1,\
> path=/tmp/test-org.qemu.guest_agent.00,server,nowait:
> Duplicate ID 'libxl-channel-1' for chardev
> 
> Besides the odd '-1' value in the id, multiple channels have the same
> id, causing qemu to fail. A simple fix is to set an uninitialized
> devid (-1) to the dev_num passed to libxl__init_console_from_channel().
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
> 
> I get the feeling that if needed devid should be set earlier, but
> this seems like the most opportune spot. Suggestions for improvements
> welcome.
> 
>   tools/libxl/libxl_console.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
> index 39d8430df8..8faf3a24f3 100644
> --- a/tools/libxl/libxl_console.c
> +++ b/tools/libxl/libxl_console.c
> @@ -401,6 +401,9 @@ int libxl__init_console_from_channel(libxl__gc *gc,
>   
>       /* Perform validation first, allocate second. */
>   
> +    if (channel->devid == -1)
> +        channel->devid = dev_num;
> +
>       if (!channel->name) {
>           LOG(ERROR, "channel %d has no name", channel->devid);
>           return ERROR_INVAL;
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxl: set channel devid when not provided by application

[Roger Pau Monné]

On Wed, Feb 07, 2018 at 08:04:10PM -0700, Jim Fehlig wrote:
> Applications like libvirt may not populate a device devid field,
> delegating that to libxl. If needed, the application can later
> retrieve the libxl-produced devid. Indeed most devices are handled
> this way in libvirt, channel devices included.
> 
> This works well when only one channel device is defined, but more
> than one results in
> 
> qemu-system-i386: -chardev socket,id=libxl-channel-1,\
> path=/tmp/test-org.qemu.guest_agent.00,server,nowait:
> Duplicate ID 'libxl-channel-1' for chardev
> 
> Besides the odd '-1' value in the id, multiple channels have the same
> id, causing qemu to fail. A simple fix is to set an uninitialized
> devid (-1) to the dev_num passed to libxl__init_console_from_channel().
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
> 
> I get the feeling that if needed devid should be set earlier, but
> this seems like the most opportune spot. Suggestions for improvements
> welcome.
> 
>  tools/libxl/libxl_console.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tools/libxl/libxl_console.c b/tools/libxl/libxl_console.c
> index 39d8430df8..8faf3a24f3 100644
> --- a/tools/libxl/libxl_console.c
> +++ b/tools/libxl/libxl_console.c
> @@ -401,6 +401,9 @@ int libxl__init_console_from_channel(libxl__gc *gc,
>  
>      /* Perform validation first, allocate second. */
>  
> +    if (channel->devid == -1)
> +        channel->devid = dev_num;

This chunk looks fine, but below in this function there's a:

console->devid = dev_num;

Which needs to be replaced with:

console->devid = channel->devid;

In this same patch.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxl: set channel devid when not provided by application

[Wei Liu]

On Wed, Feb 07, 2018 at 08:04:10PM -0700, Jim Fehlig wrote:
> Applications like libvirt may not populate a device devid field,
> delegating that to libxl. If needed, the application can later
> retrieve the libxl-produced devid. Indeed most devices are handled
> this way in libvirt, channel devices included.
> 
> This works well when only one channel device is defined, but more
> than one results in
> 
> qemu-system-i386: -chardev socket,id=libxl-channel-1,\
> path=/tmp/test-org.qemu.guest_agent.00,server,nowait:
> Duplicate ID 'libxl-channel-1' for chardev
> 
> Besides the odd '-1' value in the id, multiple channels have the same
> id, causing qemu to fail. A simple fix is to set an uninitialized
> devid (-1) to the dev_num passed to libxl__init_console_from_channel().
> 
> Signed-off-by: Jim Fehlig <jfehlig@suse.com>
> ---
> 
> I get the feeling that if needed devid should be set earlier, but
> this seems like the most opportune spot. Suggestions for improvements
> welcome.

I think this approach is fine.

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel