Re: [libvirt PATCH] qemu: support multiqueue for vdpa net device

2022-03-03 Thread Martin Kletzander

On Wed, Mar 02, 2022 at 03:15:51PM -0600, Jonathon Jongsma wrote:

On 3/2/22 3:30 AM, Martin Kletzander wrote:

On Tue, Mar 01, 2022 at 05:21:37PM -0600, Jonathon Jongsma wrote:

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2024406

Signed-off-by: Jonathon Jongsma 
---
src/conf/domain_conf.c    | 11 ++
src/qemu/qemu_domain.c    |  3 +-
.../net-vdpa-multiqueue.x86_64-latest.args    | 36 +++
.../qemuxml2argvdata/net-vdpa-multiqueue.xml  | 29 +++
tests/qemuxml2argvtest.c  |  1 +
.../net-vdpa-multiqueue.xml   | 36 +++
tests/qemuxml2xmltest.c   |  1 +
7 files changed, 116 insertions(+), 1 deletion(-)
create mode 100644
tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/net-vdpa-multiqueue.xml
create mode 100644 tests/qemuxml2xmloutdata/net-vdpa-multiqueue.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 34fec887a3..87117a2ddb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10552,6 +10552,17 @@ virDomainNetDefParseXML(virDomainXMLOption
*xmlopt,
    goto error;
    }
    def->data.vdpa.devicepath = g_steal_pointer();
+
+    if (!def->model)
+    def->model = VIR_DOMAIN_NET_MODEL_VIRTIO;
+
+    if (!virDomainNetIsVirtioModel(def)) {
+    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Wrong  'type' attribute specified "
+ "with . "
+ "vdpa requires the virtio-net* frontend"));
+    goto error;
+    }


This looks like it belongs to the verification phase as well so that it
does not reject already-existing domain XMLs.


I guess that brings up the question about whether the model should be
set here at all. It was already being set to 'virtio' in the post-parse
callback but I decided to set it here during parse because the
virtio-specific driver options (notably multiqueue in this case) don't
even get parsed unless the model is explicitly set to virtio at this point.

Note that the same behavior can also occur for other network devices.
qemuDomainDefaultNetModel() returns 'virtio' for some configurations
(e.g. s390, riscv, etc). But since the default model type is not set
until the post-parse callback, any virtio driver options that are
specified in the xml will be silently ignored for configurations that
don't include an explicit  (even if the model
defaults to virtio) because the model will be unset during parse.

If that's how it's expected to behave, then I could remove this whole
hunk and just require that the model be explicitly set to virtio in the
xml in order to use multiqueue.



I would not expect that to happen, but I can imagine that being
... fine? I guess?  I don't necessarily care about setting the model,
but the error reporting and quitting should be done in PostParse IMHO,
just to be on the safe side of things.




    break;

    case VIR_DOMAIN_NET_TYPE_BRIDGE:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index acc76c1cd6..6b61fefb8f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4523,7 +4523,8 @@ qemuDomainValidateActualNetDef(const
virDomainNetDef *net,
  actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
  actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
  actualType == VIR_DOMAIN_NET_TYPE_ETHERNET ||
-  actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)) {
+  actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER ||
+  actualType == VIR_DOMAIN_NET_TYPE_VDPA)) {
    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
   _("interface %s - multiqueue is not
supported for network interfaces of type %s"),
   macstr, virDomainNetTypeToString(actualType));


Do I get that we cannot support it for  but we do for
?  Just asking if I understand it correctly, no need to
change that.


I'm not sure, to be perfectly honest. I'm just dealing with the vdpa
device at the moment.



Yeah, let's leave it like this.






diff --git
a/tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args
b/tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args
new file mode 100644
index 00..26ef666036
--- /dev/null
+++ b/tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args
@@ -0,0 +1,36 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}'
\
+-machine 

Re: [libvirt PATCH] qemu: support multiqueue for vdpa net device

2022-03-02 Thread Jonathon Jongsma

On 3/2/22 3:30 AM, Martin Kletzander wrote:

On Tue, Mar 01, 2022 at 05:21:37PM -0600, Jonathon Jongsma wrote:

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2024406

Signed-off-by: Jonathon Jongsma 
---
src/conf/domain_conf.c    | 11 ++
src/qemu/qemu_domain.c    |  3 +-
.../net-vdpa-multiqueue.x86_64-latest.args    | 36 +++
.../qemuxml2argvdata/net-vdpa-multiqueue.xml  | 29 +++
tests/qemuxml2argvtest.c  |  1 +
.../net-vdpa-multiqueue.xml   | 36 +++
tests/qemuxml2xmltest.c   |  1 +
7 files changed, 116 insertions(+), 1 deletion(-)
create mode 100644 
tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args

create mode 100644 tests/qemuxml2argvdata/net-vdpa-multiqueue.xml
create mode 100644 tests/qemuxml2xmloutdata/net-vdpa-multiqueue.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 34fec887a3..87117a2ddb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10552,6 +10552,17 @@ virDomainNetDefParseXML(virDomainXMLOption 
*xmlopt,

    goto error;
    }
    def->data.vdpa.devicepath = g_steal_pointer();
+
+    if (!def->model)
+    def->model = VIR_DOMAIN_NET_MODEL_VIRTIO;
+
+    if (!virDomainNetIsVirtioModel(def)) {
+    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Wrong  'type' attribute specified "
+ "with . "
+ "vdpa requires the virtio-net* frontend"));
+    goto error;
+    }


This looks like it belongs to the verification phase as well so that it
does not reject already-existing domain XMLs.


I guess that brings up the question about whether the model should be 
set here at all. It was already being set to 'virtio' in the post-parse 
callback but I decided to set it here during parse because the 
virtio-specific driver options (notably multiqueue in this case) don't 
even get parsed unless the model is explicitly set to virtio at this point.


Note that the same behavior can also occur for other network devices. 
qemuDomainDefaultNetModel() returns 'virtio' for some configurations 
(e.g. s390, riscv, etc). But since the default model type is not set 
until the post-parse callback, any virtio driver options that are 
specified in the xml will be silently ignored for configurations that 
don't include an explicit  (even if the model 
defaults to virtio) because the model will be unset during parse.


If that's how it's expected to behave, then I could remove this whole 
hunk and just require that the model be explicitly set to virtio in the 
xml in order to use multiqueue.





    break;

    case VIR_DOMAIN_NET_TYPE_BRIDGE:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index acc76c1cd6..6b61fefb8f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4523,7 +4523,8 @@ qemuDomainValidateActualNetDef(const 
virDomainNetDef *net,

  actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
  actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
  actualType == VIR_DOMAIN_NET_TYPE_ETHERNET ||
-  actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)) {
+  actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER ||
+  actualType == VIR_DOMAIN_NET_TYPE_VDPA)) {
    virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
   _("interface %s - multiqueue is not 
supported for network interfaces of type %s"),

   macstr, virDomainNetTypeToString(actualType));


Do I get that we cannot support it for  but we do for
?  Just asking if I understand it correctly, no need to
change that.


I'm not sure, to be perfectly honest. I'm just dealing with the vdpa 
device at the moment.





diff --git 
a/tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args 
b/tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args

new file mode 100644
index 00..26ef666036
--- /dev/null
+++ b/tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args
@@ -0,0 +1,36 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object 
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}' 
\

+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \
+-accel tcg \
+-cpu qemu64 \
+-m 214 \
+-object 
'{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \

+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev 

Re: [libvirt PATCH] qemu: support multiqueue for vdpa net device

2022-03-02 Thread Martin Kletzander

On Tue, Mar 01, 2022 at 05:21:37PM -0600, Jonathon Jongsma wrote:

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2024406

Signed-off-by: Jonathon Jongsma 
---
src/conf/domain_conf.c| 11 ++
src/qemu/qemu_domain.c|  3 +-
.../net-vdpa-multiqueue.x86_64-latest.args| 36 +++
.../qemuxml2argvdata/net-vdpa-multiqueue.xml  | 29 +++
tests/qemuxml2argvtest.c  |  1 +
.../net-vdpa-multiqueue.xml   | 36 +++
tests/qemuxml2xmltest.c   |  1 +
7 files changed, 116 insertions(+), 1 deletion(-)
create mode 100644 tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args
create mode 100644 tests/qemuxml2argvdata/net-vdpa-multiqueue.xml
create mode 100644 tests/qemuxml2xmloutdata/net-vdpa-multiqueue.xml

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 34fec887a3..87117a2ddb 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10552,6 +10552,17 @@ virDomainNetDefParseXML(virDomainXMLOption *xmlopt,
goto error;
}
def->data.vdpa.devicepath = g_steal_pointer();
+
+if (!def->model)
+def->model = VIR_DOMAIN_NET_MODEL_VIRTIO;
+
+if (!virDomainNetIsVirtioModel(def)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+   _("Wrong  'type' attribute specified "
+ "with . "
+ "vdpa requires the virtio-net* frontend"));
+goto error;
+}


This looks like it belongs to the verification phase as well so that it
does not reject already-existing domain XMLs.


break;

case VIR_DOMAIN_NET_TYPE_BRIDGE:
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index acc76c1cd6..6b61fefb8f 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -4523,7 +4523,8 @@ qemuDomainValidateActualNetDef(const virDomainNetDef *net,
  actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
  actualType == VIR_DOMAIN_NET_TYPE_DIRECT ||
  actualType == VIR_DOMAIN_NET_TYPE_ETHERNET ||
-  actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER)) {
+  actualType == VIR_DOMAIN_NET_TYPE_VHOSTUSER ||
+  actualType == VIR_DOMAIN_NET_TYPE_VDPA)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
   _("interface %s - multiqueue is not supported for network 
interfaces of type %s"),
   macstr, virDomainNetTypeToString(actualType));


Do I get that we cannot support it for  but we do for
?  Just asking if I understand it correctly, no need to
change that.


diff --git a/tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args 
b/tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args
new file mode 100644
index 00..26ef666036
--- /dev/null
+++ b/tests/qemuxml2argvdata/net-vdpa-multiqueue.x86_64-latest.args
@@ -0,0 +1,36 @@
+LC_ALL=C \
+PATH=/bin \
+HOME=/tmp/lib/domain--1-QEMUGuest1 \
+USER=test \
+LOGNAME=test \
+XDG_DATA_HOME=/tmp/lib/domain--1-QEMUGuest1/.local/share \
+XDG_CACHE_HOME=/tmp/lib/domain--1-QEMUGuest1/.cache \
+XDG_CONFIG_HOME=/tmp/lib/domain--1-QEMUGuest1/.config \
+/usr/bin/qemu-system-x86_64 \
+-name guest=QEMUGuest1,debug-threads=on \
+-S \
+-object 
'{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/tmp/lib/domain--1-QEMUGuest1/master-key.aes"}'
 \
+-machine pc,usb=off,dump-guest-core=off,memory-backend=pc.ram \
+-accel tcg \
+-cpu qemu64 \
+-m 214 \
+-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":224395264}' \
+-overcommit mem-lock=off \
+-smp 1,sockets=1,cores=1,threads=1 \
+-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \
+-display none \
+-no-user-config \
+-nodefaults \
+-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \
+-mon chardev=charmonitor,id=monitor,mode=control \
+-rtc base=utc \
+-no-shutdown \
+-no-acpi \
+-boot strict=on \
+-device 
'{"driver":"piix3-usb-uhci","id":"usb","bus":"pci.0","addr":"0x1.0x2"}' \
+-add-fd set=0,fd=1732,opaque=net0-vdpa \
+-netdev vhost-vdpa,vhostdev=/dev/fdset/0,id=hostnet0 \
+-device 
'{"driver":"virtio-net-pci","mq":true,"vectors":6,"netdev":"hostnet0","id":"net0","mac":"52:54:00:95:db:c0","bus":"pci.0","addr":"0x2"}'
 \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-sandbox 
on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \
+-msg timestamp=on
diff --git a/tests/qemuxml2argvdata/net-vdpa-multiqueue.xml 
b/tests/qemuxml2argvdata/net-vdpa-multiqueue.xml
new file mode 100644
index 00..2e38c6f976
--- /dev/null
+++ b/tests/qemuxml2argvdata/net-vdpa-multiqueue.xml
@@ -0,0 +1,29 @@
+
+  QEMUGuest1
+  c7a5fdbd-edaf-9455-926a-d65c16db1809
+  219136
+  219136
+  1
+  
+hvm
+
+  
+  
+  destroy
+  restart
+  destroy
+  
+/usr/bin/qemu-system-x86_64
+
+
+
+
+  
+  
+  
+
+
+
+
+  
+
diff