[libvirt] [PATCH 2/4] Introducing assigned='yes|no' hostdev attribute

Daniel Henrique Barboza posted 4 patches 5 years, 1 month ago
[libvirt] [PATCH 2/4] Introducing assigned='yes|no' hostdev attribute
Posted by Daniel Henrique Barboza 5 years, 1 month ago
The idea of having an attribute that declares that a hostdev
is not to be assigned to the guest seems counterintuitive, but
it has a niche use with multifunction PCI devices.

The current use of this kind of device in Libvirt is to declare
all of the functions in the XML. This is not ideal though - some
devices might have functions that are security sensitive to
be shared to guests, but at the same time the user might want
to passthrough the other functions to a guest. This is what
we call 'partial assignment'. Libvirt does not have proper
support for this scenario: when using managed='yes', this
partial assignment will cause the code inside virhostdev.c
to not detach all the IOMMU devices - given that we do not
want all of them in the guest - and then we face a QEMU
error because we didn't detached the whole IOMMU.

An idea was discussed in [1] where Libvirt would automatically
detach all the IOMMU devices in case any multifunction PCI
hostdev is used in the guest, but this idea was discarded
because removing user agency in this case is undesirable. The
user must be aware of all the functions that are going to be
detached from the host, even if they're not being assigned
to the guest, to avoid scenarios in which Libvirt "all of a
sudden" detaches something that the user didn't want to.

This patch implements a new attribute called 'assigned' that
indicates whether a hostdev is going to be assigned to the
guest or not. To keep its use less intrusive for every other
hostdev that does not need such control, the attribute can
only be set by PCI multifunction devices. For existing
domains prior to this change, assign='yes' will be
implied. If the user decides not to assign a specific
function to the guest using assign='no', the <address>
field becomes unavailable and throws a parsing error
if used.

In the next patch we'll use this attribute to avoid assigning
an assigned='no' device to QEMU at boot.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---

Note: the changes in the .config files were done because
the existing .config data files weren't being recognized
as multifunction PCI devices after we started reading the
PCI header.



 docs/schemas/domaincommon.rng                 |   5 +++
 src/conf/domain_conf.c                        |  36 +++++++++++++++++-
 src/conf/domain_conf.h                        |   1 +
 .../hostdev-pci-multifunction.args            |   7 ++--
 .../hostdev-pci-multifunction.xml             |   6 +++
 .../hostdev-pci-multifunction.xml             |  23 +++++++----
 .../qemuxml2xmloutdata/pseries-hostdevs-1.xml |   4 +-
 .../qemuxml2xmloutdata/pseries-hostdevs-2.xml |   4 +-
 .../qemuxml2xmloutdata/pseries-hostdevs-3.xml |   4 +-
 tests/virpcitestdata/0005-90-01.1.config      | Bin 256 -> 256 bytes
 tests/virpcitestdata/0005-90-01.2.config      | Bin 256 -> 256 bytes
 tests/virpcitestdata/0005-90-01.3.config      | Bin 0 -> 256 bytes
 tools/virsh-domain.c                          |   5 +++
 13 files changed, 76 insertions(+), 19 deletions(-)
 create mode 100644 tests/virpcitestdata/0005-90-01.3.config

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 40eb4a2d75..d1a5c051e3 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4629,6 +4629,11 @@
     <attribute name="type">
       <value>pci</value>
     </attribute>
+    <optional>
+      <attribute name="assigned">
+        <ref name="virYesNo"/>
+      </attribute>
+    </optional>
     <interleave>
       <optional>
         <element name="driver">
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a53cd6a725..8d80824a0a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7755,7 +7755,6 @@ virDomainHostdevSubsysPCIDefParseXML(xmlNodePtr node,
             if (virXMLNodeNameEqual(cur, "address")) {
                 virPCIDeviceAddressPtr addr =
                     &def->source.subsys.u.pci.addr;
-
                 if (virPCIDeviceAddressParseXML(cur, addr) < 0)
                     goto out;
             } else if ((flags & VIR_DOMAIN_DEF_PARSE_STATUS) &&
@@ -8147,7 +8146,7 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
                                   virDomainHostdevDefPtr def,
                                   unsigned int flags)
 {
-    xmlNodePtr sourcenode;
+    xmlNodePtr sourcenode, addressnode;
     int backend;
     virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci;
     virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi;
@@ -8159,6 +8158,8 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
     VIR_AUTOFREE(char *) backendStr = NULL;
     VIR_AUTOFREE(char *) model = NULL;
     VIR_AUTOFREE(char *) display = NULL;
+    VIR_AUTOFREE(char *) assigned = NULL;
+
 
     /* @managed can be read from the xml document - it is always an
      * attribute of the toplevel element, no matter what type of
@@ -8289,6 +8290,32 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
         if (virDomainHostdevSubsysPCIDefParseXML(sourcenode, def, flags) < 0)
             return -1;
 
+        /* @assigned can only be set for multifunction PCI devices.
+         * In case the attribute is missing, always assume
+         * assigned = true.
+         */
+        def->assigned = true;
+
+        if ((assigned = virXMLPropString(node, "assigned")) != NULL) {
+            if (!virHostdevIsPCIMultifunctionDevice(def)) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("'assigned' can only be set for multifunction "
+                                 "PCI devices"));
+                return -1;
+            }
+
+            if (STREQ(assigned, "no"))
+                def->assigned = false;
+
+            if ((addressnode = virXPathNode("./address", ctxt)) &&
+                !def->assigned) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                               _("unable to set <address> element "
+                                 "for assign='no' hostdev"));
+                return -1;
+            }
+        }
+
         backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT;
         if ((backendStr = virXPathString("string(./driver/@name)", ctxt)) &&
             (((backend = virDomainHostdevSubsysPCIBackendTypeFromString(backendStr)) < 0) ||
@@ -27254,6 +27281,11 @@ virDomainHostdevDefFormat(virBufferPtr buf,
         virBufferAsprintf(buf, " managed='%s'",
                           def->managed ? "yes" : "no");
 
+        if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI &&
+            virHostdevIsPCIMultifunctionDevice(def))
+            virBufferAsprintf(buf, " assigned='%s'",
+                              def->assigned ? "yes" : "no");
+
         if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI &&
             scsisrc->sgio)
             virBufferAsprintf(buf, " sgio='%s'",
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 2884af49d8..e2f6f640b9 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -343,6 +343,7 @@ struct _virDomainHostdevDef {
     bool missing;
     bool readonly;
     bool shareable;
+    bool assigned;
     union {
         virDomainHostdevSubsys subsys;
         virDomainHostdevCaps caps;
diff --git a/tests/qemuxml2argvdata/hostdev-pci-multifunction.args b/tests/qemuxml2argvdata/hostdev-pci-multifunction.args
index d8690c010b..3cef177c00 100644
--- a/tests/qemuxml2argvdata/hostdev-pci-multifunction.args
+++ b/tests/qemuxml2argvdata/hostdev-pci-multifunction.args
@@ -30,6 +30,7 @@ server,nowait \
 -device vfio-pci,host=0001:01:00.0,id=hostdev2,bus=pci.0,addr=0x5 \
 -device vfio-pci,host=0005:90:01.2,id=hostdev3,bus=pci.0,addr=0x6 \
 -device vfio-pci,host=0005:90:01.3,id=hostdev4,bus=pci.0,addr=0x7 \
--device vfio-pci,host=0000:06:12.1,id=hostdev5,bus=pci.0,addr=0x8 \
--device vfio-pci,host=0000:06:12.2,id=hostdev6,bus=pci.0,addr=0x9 \
--device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xa
+-device vfio-pci,host=0000:06:12.0,id=hostdev5,bus=pci.0,addr=0x8 \
+-device vfio-pci,host=0000:06:12.1,id=hostdev6,bus=pci.0,addr=0x9 \
+-device vfio-pci,host=0000:06:12.2,id=hostdev7,bus=pci.0,addr=0xa \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0xb
diff --git a/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml b/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml
index 06c889c64d..f4813961b3 100644
--- a/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml
+++ b/tests/qemuxml2argvdata/hostdev-pci-multifunction.xml
@@ -43,6 +43,12 @@
         <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/>
       </source>
     </hostdev>
+    <hostdev mode='subsystem' type='pci' managed='yes'>
+      <driver name='vfio'/>
+      <source>
+        <address domain='0x0000' bus='0x06' slot='0x12' function='0x0'/>
+      </source>
+    </hostdev>
     <hostdev mode='subsystem' type='pci' managed='yes'>
       <driver name='vfio'/>
       <source>
diff --git a/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml b/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml
index 52ed86e305..079e0513c1 100644
--- a/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml
+++ b/tests/qemuxml2xmloutdata/hostdev-pci-multifunction.xml
@@ -23,35 +23,35 @@
     <controller type='pci' index='0' model='pci-root'/>
     <input type='mouse' bus='ps2'/>
     <input type='keyboard' bus='ps2'/>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0005' bus='0x90' slot='0x01' function='0x0'/>
       </source>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
     </hostdev>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/>
       </source>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
     </hostdev>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/>
       </source>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
     </hostdev>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0005' bus='0x90' slot='0x01' function='0x2'/>
       </source>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
     </hostdev>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0005' bus='0x90' slot='0x01' function='0x3'/>
@@ -61,19 +61,26 @@
     <hostdev mode='subsystem' type='pci' managed='yes'>
       <driver name='vfio'/>
       <source>
-        <address domain='0x0000' bus='0x06' slot='0x12' function='0x1'/>
+        <address domain='0x0000' bus='0x06' slot='0x12' function='0x0'/>
       </source>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0'/>
     </hostdev>
     <hostdev mode='subsystem' type='pci' managed='yes'>
       <driver name='vfio'/>
       <source>
-        <address domain='0x0000' bus='0x06' slot='0x12' function='0x2'/>
+        <address domain='0x0000' bus='0x06' slot='0x12' function='0x1'/>
       </source>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
     </hostdev>
-    <memballoon model='virtio'>
+    <hostdev mode='subsystem' type='pci' managed='yes'>
+      <driver name='vfio'/>
+      <source>
+        <address domain='0x0000' bus='0x06' slot='0x12' function='0x2'/>
+      </source>
       <address type='pci' domain='0x0000' bus='0x00' slot='0x0a' function='0x0'/>
+    </hostdev>
+    <memballoon model='virtio'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x0b' function='0x0'/>
     </memballoon>
   </devices>
 </domain>
diff --git a/tests/qemuxml2xmloutdata/pseries-hostdevs-1.xml b/tests/qemuxml2xmloutdata/pseries-hostdevs-1.xml
index e77a060a38..94472b38e4 100644
--- a/tests/qemuxml2xmloutdata/pseries-hostdevs-1.xml
+++ b/tests/qemuxml2xmloutdata/pseries-hostdevs-1.xml
@@ -35,14 +35,14 @@
       </source>
       <address type='pci' domain='0x0000' bus='0x01' slot='0x01' function='0x0'/>
     </interface>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/>
       </source>
       <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/>
     </hostdev>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/>
diff --git a/tests/qemuxml2xmloutdata/pseries-hostdevs-2.xml b/tests/qemuxml2xmloutdata/pseries-hostdevs-2.xml
index cfa395b001..f2fe94d2fb 100644
--- a/tests/qemuxml2xmloutdata/pseries-hostdevs-2.xml
+++ b/tests/qemuxml2xmloutdata/pseries-hostdevs-2.xml
@@ -30,14 +30,14 @@
       <model name='spapr-pci-host-bridge'/>
       <target index='2'/>
     </controller>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/>
       </source>
       <address type='pci' domain='0x0000' bus='0x01' slot='0x02' function='0x0'/>
     </hostdev>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0005' bus='0x90' slot='0x01' function='0x0'/>
diff --git a/tests/qemuxml2xmloutdata/pseries-hostdevs-3.xml b/tests/qemuxml2xmloutdata/pseries-hostdevs-3.xml
index f91959b805..0893ecd887 100644
--- a/tests/qemuxml2xmloutdata/pseries-hostdevs-3.xml
+++ b/tests/qemuxml2xmloutdata/pseries-hostdevs-3.xml
@@ -27,14 +27,14 @@
       <model name='spapr-pci-host-bridge'/>
       <target index='2'/>
     </controller>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0001' bus='0x01' slot='0x00' function='0x0'/>
       </source>
       <address type='pci' domain='0x0000' bus='0x02' slot='0x01' function='0x0'/>
     </hostdev>
-    <hostdev mode='subsystem' type='pci' managed='yes'>
+    <hostdev mode='subsystem' type='pci' managed='yes' assigned='yes'>
       <driver name='vfio'/>
       <source>
         <address domain='0x0001' bus='0x01' slot='0x00' function='0x1'/>
diff --git a/tests/virpcitestdata/0005-90-01.1.config b/tests/virpcitestdata/0005-90-01.1.config
index beee76534041a7020c08ae9ac03d9a349c6ea12e..a60599bd342d3ebcdc7b8367ca36ad337f602fde 100644
GIT binary patch
delta 44
ycmZo*YG4vE7BFRCV-R3+7GUOK;Amg~f~JXq5)*X<85t+qEn;Cc-jFjfPzC^<7YL>R

delta 39
ucmZo*YG4vE7BFRCV-R3+7GUOK;9y{25MXGU7$`AON05<eqTQm22?_viD+cla

diff --git a/tests/virpcitestdata/0005-90-01.2.config b/tests/virpcitestdata/0005-90-01.2.config
index cfd72e4d7165bff2751ecbdc570ce7f1e0646226..a60599bd342d3ebcdc7b8367ca36ad337f602fde 100644
GIT binary patch
literal 256
zcmXpOFlAt45MXi^VCG@qXkY+>CJ=!Q7z5RUfCHEW5{!&mj0{Y5Fz#TaS&cX3;ByxM
DCDjDB

literal 256
zcmXpOc)-BMAi%_;z|6zo!oa|wz|aIFu>xbDS`csmlR$!5K#7rosSd`)Mk^@TV-u#E
T7_0Gy9FS#<5E~CbC<F-rZiWW}

diff --git a/tests/virpcitestdata/0005-90-01.3.config b/tests/virpcitestdata/0005-90-01.3.config
new file mode 100644
index 0000000000000000000000000000000000000000..a60599bd342d3ebcdc7b8367ca36ad337f602fde
GIT binary patch
literal 256
zcmXpOFlAt45MXi^VCG@qXkY+>CJ=!Q7z5RUfCHEW5{!&mj0{Y5Fz#TaS&cX3;ByxM
DCDjDB

literal 0
HcmV?d00001

diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index fbfdc09c0d..1453d7f024 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -858,6 +858,11 @@ static const vshCmdOptDef opts_attach_interface[] = {
      .type = VSH_OT_BOOL,
      .help = N_("libvirt will automatically detach/attach the device from/to host")
     },
+    {.name = "assigned",
+     .type = VSH_OT_BOOL,
+     .help = N_("hostdev virtual function will be assigned to the guest "
+                "(multifunction PCI device only)")
+    },
     {.name = NULL}
 };
 
-- 
2.21.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list