chardev: add new socket fd parameter for unix socket

There has been comments on this patch, but i forgot adding this patch to the list, just forward it again.

When manage VMs via libvirt, qemu ofter runs with limited permission, thus qemu can't create a file/socket, this patch is to add a new parameter 'sockfd' to accept fd opened and passed in from libvirt.

Signed-off-by Wei Xu <[email protected]>

---
 qapi-schema.json | 3 ++-
 qemu-char.c      | 3 +++
 2 files changed, 5 insertions(+), 1 deletion(-)

qapi-schema.json +0 -0

index 8483bdf..e9f0268 100644

Quoted 18 lines · Wei Xu

There have been comments on this patch, but i forgot adding this patch to the list, just forward it again.

When manage VMs via libvirt, qemu ofter runs with limited permission, thus qemu can't create a file/socket, this patch is to add a new parameter 'sockfd' to accept fd opened and passed in from libvirt.

Signed-off-by Wei Xu <[email protected]>

---
  qapi-schema.json | 3 ++-
  qemu-char.c      | 3 +++
  2 files changed, 5 insertions(+), 1 deletion(-)

qapi-schema.json +0 -0

index 8483bdf..e9f0268 100644

qapi-schema.json line 2925

Quoted 6 lines

qapi-schema.json +2 -1

@@ -2921,7 +2921,8 @@
  ##
  { 'struct': 'UnixSocketAddress',
    'data': {
-    'path': 'str' } }
+    'path': 'str',
+    'sockfd': 'int32' } }

Missing documentation.

This makes the new 'sockfd' parameter mandatory, but SocketAddress is an input type. This is not backwards compatible. At best, you'd want to make it optional, but I'm not even convinced you want to add it, since we already can use the magic /dev/fdset/nnn in 'path' to pass an arbitrary fd if the underlying code uses qemu_open().

Quoted 29 lines · Eric Blake

Quoted 19 lines · Wei Xu

There have been comments on this patch, but i forgot adding this patch to
the list, just forward it again.

When manage VMs via libvirt, qemu ofter runs with limited permission, thus qemu can't create a file/socket, this patch is to add a new parameter 'sockfd' to accept fd opened and passed in from libvirt.

Signed-off-by Wei Xu <[email protected]>

---
  qapi-schema.json | 3 ++-
  qemu-char.c      | 3 +++
  2 files changed, 5 insertions(+), 1 deletion(-)

qapi-schema.json +0 -0

index 8483bdf..e9f0268 100644

qapi-schema.json line 2925

Quoted 6 lines

qapi-schema.json +2 -1

@@ -2921,7 +2921,8 @@
  ##
  { 'struct': 'UnixSocketAddress',
    'data': {
-    'path': 'str' } }
+    'path': 'str',
+    'sockfd': 'int32' } }

Missing documentation.

Quoted 6 lines

This makes the new 'sockfd' parameter mandatory, but SocketAddress is an input type. This is not backwards compatible. At best, you'd want to make it optional, but I'm not even convinced you want to add it, since we already can use the magic /dev/fdset/nnn in 'path' to pass an arbitrary fd if the underlying code uses qemu_open().

Thanks for commenting it again, i was going to forward it to the list and ask some questions.:)

Actually i'm going to try the magic way as you suggested, just a few questions about that.

Command line change should be like this according to my understanding.

Current command line:
-chardev socket,id=char0,path=/var/run/openvswitch/vhost-user1,server

New command line:
qemu-kvm -add-fd fd=3,set=2,opaque="/var/run/openvswitch/vhost-user1" -chardev socket,id=char0,path=/dev/fdset/3,server

Q1. The 'sockfd' is not used anymore, but looks the 'path' parameter is still mandatory AFAIK, because it's a unix domain socket, which is different with a network tcp/udp socket, it works like a pipe file for local communication only, and the 'path' parameter is a must-have condition before a real connect could be established, it needs a bind() operation to a specific path before either connect() or listen(), this is caused by libvirt only takes the responsibility to creates a socket and pass the 'fd' in only, there is nothing more about the bind(), thus i think qemu will have to bind() it by itself, i'm thinking maybe 'opaque' can be used for this case.

Q2. Do you know how can i test it? i'm going to fork a process first and create a socket like libvirt, then exec qemu and pass it in, just wondering how can i map it to '/dev/fdset/' directory after created the socket?

Q3.
-------------------------------------------------------------------
Daniel's comment before:
'Path' refers to a UNIX domain socket path, so the code won't be using qemu_open() - it'll be using the sockets APIs to open/create a UNIX socket. I don't think qemu_open() would even work with a socket FD, since it'll be trying to set various modes on the FD via fcntl() that don't work with sockets AFAIK
-------------------------------------------------------------------
Seems what i should call is qemu_socket() to fill in qemu_open(), i should check if it's started as '/dev/fdset' like in qemu_open(), and just pick the 'fd' up, is this enough? should i check the modes?

Thanks,
Wei

Quoted 28 lines · Eric Blake

Quoted 18 lines · Wei Xu

There have been comments on this patch, but i forgot adding this patch to the list, just forward it again.

When manage VMs via libvirt, qemu ofter runs with limited permission, thus qemu can't create a file/socket, this patch is to add a new parameter 'sockfd' to accept fd opened and passed in from libvirt.

Signed-off-by Wei Xu <[email protected]>

---
  qapi-schema.json | 3 ++-
  qemu-char.c      | 3 +++
  2 files changed, 5 insertions(+), 1 deletion(-)

qapi-schema.json +0 -0

index 8483bdf..e9f0268 100644

qapi-schema.json line 2925

Quoted 6 lines

qapi-schema.json +2 -1

@@ -2921,7 +2921,8 @@
  ##
  { 'struct': 'UnixSocketAddress',
    'data': {
-    'path': 'str' } }
+    'path': 'str',
+    'sockfd': 'int32' } }

Missing documentation.

Quoted 6 lines

This makes the new 'sockfd' parameter mandatory, but SocketAddress is an input type. This is not backwards compatible. At best, you'd want to make it optional, but I'm not even convinced you want to add it, since we already can use the magic /dev/fdset/nnn in 'path' to pass an arbitrary fd if the underlying code uses qemu_open().

Thanks for commenting it again, i was going to forward it to the list and ask some questions.:)

Actually i'm going to try the magic way as you suggested, just a few questions about that.

Command line change should be like this according to my understanding.

Current command line:
-chardev socket,id=char0,path=/var/run/openvswitch/vhost-user1,server

New command line:
qemu-kvm -add-fd fd=3,set=2,opaque="/var/run/openvswitch/vhost-user1" -chardev socket,id=char0,path=/dev/fdset/3,server

Q1. The 'sockfd' is not used anymore, but looks the 'path' parameter is still mandatory AFAIK, because it's a unix domain socket, which is different with a network tcp/udp socket, it works like a pipe file for local communication only, and the 'path' parameter is a must-have condition before a real connect could be established, it needs a bind() operation to a specific path before either connect() or listen(), this is caused by libvirt only takes the responsibility to creates a socket and pass the 'fd' in only, there is nothing more about the bind(), thus i think qemu will have to bind() it by itself, i'm thinking maybe 'opaque' can be used for this case.

Q2. Do you know how can i test it? i'm going to fork a process first and create a socket like libvirt, then exec qemu and pass it in, just wondering how can i map it to '/dev/fdset/' directory after created the socket?

Q3.
-------------------------------------------------------------------
Daniel's comment before:
'Path' refers to a UNIX domain socket path, so the code won't be using qemu_open() - it'll be using the sockets APIs to open/create a UNIX socket. I don't think qemu_open() would even work with a socket FD, since it'll be trying to set various modes on the FD via fcntl() that don't work with sockets AFAIK
-------------------------------------------------------------------
Seems what i should call is qemu_socket() to fill in qemu_open(), i should check if it's started as '/dev/fdset' like in qemu_open(), and just pick the 'fd' up, is this enough? should i check the modes?

Thanks,
Wei

Quoted 51 lines · Wei Xu

Quoted 28 lines · Eric Blake

Quoted 18 lines · Wei Xu

There have been comments on this patch, but i forgot adding this patch to the list, just forward it again.

When manage VMs via libvirt, qemu ofter runs with limited permission, thus qemu can't create a file/socket, this patch is to add a new parameter 'sockfd' to accept fd opened and passed in from libvirt.

Signed-off-by Wei Xu <[email protected]>

---
   qapi-schema.json | 3 ++-
   qemu-char.c      | 3 +++
   2 files changed, 5 insertions(+), 1 deletion(-)

qapi-schema.json +0 -0

index 8483bdf..e9f0268 100644

qapi-schema.json line 2925

Quoted 6 lines

qapi-schema.json +2 -1

@@ -2921,7 +2921,8 @@
   ##
   { 'struct': 'UnixSocketAddress',
     'data': {
-    'path': 'str' } }
+    'path': 'str',
+    'sockfd': 'int32' } }

Missing documentation.

Quoted 6 lines

This makes the new 'sockfd' parameter mandatory, but SocketAddress is an input type. This is not backwards compatible. At best, you'd want to make it optional, but I'm not even convinced you want to add it, since we already can use the magic /dev/fdset/nnn in 'path' to pass an arbitrary fd if the underlying code uses qemu_open().

Thanks for commenting it again, i was going to forward it to the list and ask some questions.:)

Actually i'm going to try the magic way as you suggested, just a few questions about that.

Command line change should be like this according to my understanding.

Current command line:
-chardev socket,id=char0,path=/var/run/openvswitch/vhost-user1,server

New command line:
qemu-kvm -add-fd fd=3,set=2,opaque="/var/run/openvswitch/vhost-user1" -chardev socket,id=char0,path=/dev/fdset/3,server

Quoted 9 lines

Q1. The 'sockfd' is not used anymore, but looks the 'path' parameter is still mandatory AFAIK, because it's a unix domain socket, which is different with a network tcp/udp socket, it works like a pipe file for local communication only, and the 'path' parameter is a must-have condition before a real connect could be established, it needs a bind() operation to a specific path before either connect() or listen(), this is caused by libvirt only takes the responsibility to creates a socket and pass the 'fd' in only, there is nothing more about the bind(), thus i think qemu will have to bind() it by itself, i'm thinking maybe 'opaque' can be used for this case.

I think libvirt will have to bind it.
Passing in an unbound socket wouldn't make sense.

Quoted 5 lines

Q2. Do you know how can i test it? i'm going to fork a process first and create a socket like libvirt, then exec qemu and pass it in, just wondering how can i map it to '/dev/fdset/' directory after created the socket?

/dev/fdset/ is QEMU syntax to pass fd numbers where path is normally used.

Quoted 16 lines

Q3.
-------------------------------------------------------------------
Daniel's comment before:
'Path' refers to a UNIX domain socket path, so the code won't be using qemu_open() - it'll be using the sockets APIs to open/create a UNIX socket. I don't think qemu_open() would even work with a socket FD, since it'll be trying to set various modes on the FD via fcntl() that don't work with sockets AFAIK
-------------------------------------------------------------------
Seems what i should call is qemu_socket() to fill in qemu_open(), i should check if it's started as '/dev/fdset' like in qemu_open(), and just pick the 'fd' up, is this enough? should i check the modes?

Thanks,
Wei