1. Patchew
  2. Libvirt
  3. View series

[libvirt] [PATCH v2] Generate unique socket file

Scott Garfinkle posted 1 patch 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1500993230-41108-1-git-send-email-seg@us.ibm.com
src/qemu/qemu_domain.c                             | 24 +++++++++++++++-------
.../qemuxml2argv-channel-virtio-unix.args          |  2 +-
2 files changed, 18 insertions(+), 8 deletions(-)
[libvirt] [PATCH v2] Generate unique socket file
Posted by Scott Garfinkle 6 years, 9 months ago 
It's possible to have more than one unnamed virtio-serial unix channel.
We need to generate a unique name for each channel. Currently, we use
".../unknown.sock" for all of them. Better practice would be to specify
an explicit target path name; however, in the absence of that, we need
uniqueness in the names we generate internally.

Before the changes we'd get /var/lib/libvirt/qemu/channel/target/unknown.sock
for each instance of
    <channel type='unix'>
        <source mode='bind'/>
        <target type='virtio'/>
    </channel>

Now, we get vioser-00-00-01.sock, vioser-00-00-02.sock, etc.

Changes from v1: new socket name

Signed-off-by: Scott Garfinkle <seg@us.ibm.com>

---
 src/qemu/qemu_domain.c                             | 24 +++++++++++++++-------
 .../qemuxml2argv-channel-virtio-unix.args          |  2 +-
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 78e75f1..7a9958d 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -7178,18 +7178,28 @@ int
 qemuDomainPrepareChannel(virDomainChrDefPtr channel,
                          const char *domainChannelTargetDir)
 {
-    if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
-        channel->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
-        !channel->source->data.nix.path) {
+    if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO ||
+        channel->source->type != VIR_DOMAIN_CHR_TYPE_UNIX ||
+        channel->source->data.nix.path)
+            return 0;
+
+    if (channel->target.name) {
         if (virAsprintf(&channel->source->data.nix.path,
                         "%s/%s", domainChannelTargetDir,
-                        channel->target.name ? channel->target.name
-                        : "unknown.sock") < 0)
+                        channel->target.name) < 0)
+            return -1;
+    } else {    // Generate a unique name
+        if (virAsprintf(&channel->source->data.nix.path,
+                        "%s/vioser-%02d-%02d-%02d.sock",
+                        domainChannelTargetDir,
+                        channel->info.addr.vioserial.controller,
+                        channel->info.addr.vioserial.bus,
+                        channel->info.addr.vioserial.port) < 0)
             return -1;
-
-        channel->source->data.nix.listen = true;
     }
 
+    channel->source->data.nix.listen = true;
+
     return 0;
 }
 
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args
index 2b72965..8e0452a 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args
@@ -29,7 +29,7 @@ path=/tmp/channel/domain--1-QEMUGuest1/org.qemu.guest_agent.0,server,nowait \
 -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,\
 id=channel0,name=org.qemu.guest_agent.0 \
 -chardev socket,id=charchannel1,\
-path=/tmp/channel/domain--1-QEMUGuest1/unknown.sock,server,nowait \
+path=/tmp/channel/domain--1-QEMUGuest1/vioser-00-00-02.sock,server,nowait \
 -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,\
 id=channel1 \
 -chardev socket,id=charchannel2,path=/tmp/channel/domain--1-QEMUGuest1/ble,\
-- 
1.8.3.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Generate unique socket file
Posted by Martin Kletzander 6 years, 9 months ago 
On Tue, Jul 25, 2017 at 09:33:50AM -0500, Scott Garfinkle wrote:
>It's possible to have more than one unnamed virtio-serial unix channel.
>We need to generate a unique name for each channel. Currently, we use
>".../unknown.sock" for all of them. Better practice would be to specify
>an explicit target path name; however, in the absence of that, we need
>uniqueness in the names we generate internally.
>
>Before the changes we'd get /var/lib/libvirt/qemu/channel/target/unknown.sock
>for each instance of
>    <channel type='unix'>
>        <source mode='bind'/>
>        <target type='virtio'/>
>    </channel>
>
>Now, we get vioser-00-00-01.sock, vioser-00-00-02.sock, etc.
>
>Changes from v1: new socket name
>
>Signed-off-by: Scott Garfinkle <seg@us.ibm.com>
>
>---
> src/qemu/qemu_domain.c                             | 24 +++++++++++++++-------
> .../qemuxml2argv-channel-virtio-unix.args          |  2 +-
> 2 files changed, 18 insertions(+), 8 deletions(-)
>
>diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>index 78e75f1..7a9958d 100644
>--- a/src/qemu/qemu_domain.c
>+++ b/src/qemu/qemu_domain.c
>@@ -7178,18 +7178,28 @@ int
> qemuDomainPrepareChannel(virDomainChrDefPtr channel,
>                          const char *domainChannelTargetDir)
> {
>-    if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
>-        channel->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
>-        !channel->source->data.nix.path) {
>+    if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO ||
>+        channel->source->type != VIR_DOMAIN_CHR_TYPE_UNIX ||
>+        channel->source->data.nix.path)
>+            return 0;
>+

Apart from the indentation right here, it looks fine.  I'll fix this up
and push it in a while.

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Generate unique socket file
Posted by Pavel Hrdina 6 years, 9 months ago 
On Tue, Jul 25, 2017 at 09:33:50AM -0500, Scott Garfinkle wrote:
> It's possible to have more than one unnamed virtio-serial unix channel.
> We need to generate a unique name for each channel. Currently, we use
> ".../unknown.sock" for all of them. Better practice would be to specify
> an explicit target path name; however, in the absence of that, we need
> uniqueness in the names we generate internally.
> 
> Before the changes we'd get /var/lib/libvirt/qemu/channel/target/unknown.sock
> for each instance of
>     <channel type='unix'>
>         <source mode='bind'/>
>         <target type='virtio'/>
>     </channel>
> 
> Now, we get vioser-00-00-01.sock, vioser-00-00-02.sock, etc.
> 
> Changes from v1: new socket name
> 
> Signed-off-by: Scott Garfinkle <seg@us.ibm.com>
> 
> ---
>  src/qemu/qemu_domain.c                             | 24 +++++++++++++++-------
>  .../qemuxml2argv-channel-virtio-unix.args          |  2 +-
>  2 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 78e75f1..7a9958d 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -7178,18 +7178,28 @@ int
>  qemuDomainPrepareChannel(virDomainChrDefPtr channel,
>                           const char *domainChannelTargetDir)
>  {
> -    if (channel->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
> -        channel->source->type == VIR_DOMAIN_CHR_TYPE_UNIX &&
> -        !channel->source->data.nix.path) {
> +    if (channel->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO ||
> +        channel->source->type != VIR_DOMAIN_CHR_TYPE_UNIX ||
> +        channel->source->data.nix.path)
> +            return 0;
> +
> +    if (channel->target.name) {
>          if (virAsprintf(&channel->source->data.nix.path,
>                          "%s/%s", domainChannelTargetDir,
> -                        channel->target.name ? channel->target.name
> -                        : "unknown.sock") < 0)
> +                        channel->target.name) < 0)
> +            return -1;
> +    } else {    // Generate a unique name

We don't allow this type of comment, since this patch is already pushed,
I'll fix it.

Pavel

> +        if (virAsprintf(&channel->source->data.nix.path,
> +                        "%s/vioser-%02d-%02d-%02d.sock",
> +                        domainChannelTargetDir,
> +                        channel->info.addr.vioserial.controller,
> +                        channel->info.addr.vioserial.bus,
> +                        channel->info.addr.vioserial.port) < 0)
>              return -1;
> -
> -        channel->source->data.nix.listen = true;
>      }
>  
> +    channel->source->data.nix.listen = true;
> +
>      return 0;
>  }
>  
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args
> index 2b72965..8e0452a 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-unix.args
> @@ -29,7 +29,7 @@ path=/tmp/channel/domain--1-QEMUGuest1/org.qemu.guest_agent.0,server,nowait \
>  -device virtserialport,bus=virtio-serial0.0,nr=1,chardev=charchannel0,\
>  id=channel0,name=org.qemu.guest_agent.0 \
>  -chardev socket,id=charchannel1,\
> -path=/tmp/channel/domain--1-QEMUGuest1/unknown.sock,server,nowait \
> +path=/tmp/channel/domain--1-QEMUGuest1/vioser-00-00-02.sock,server,nowait \
>  -device virtserialport,bus=virtio-serial0.0,nr=2,chardev=charchannel1,\
>  id=channel1 \
>  -chardev socket,id=charchannel2,path=/tmp/channel/domain--1-QEMUGuest1/ble,\
> -- 
> 1.8.3.1
> 
> --
> libvir-list mailing list
> libvir-list@redhat.com
> https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.