1. Patchew
  2. Libvirt
  3. [libvirt] [PATCHv2 0/7] qemu: add vhost-vsock-pci support
  4. View patch

[libvirt] [PATCHv2 3/7] conf: introduce <vsock> element

Ján Tomko posted 7 patches 6 years, 11 months ago
[libvirt] [PATCHv2 3/7] conf: introduce <vsock> element
Posted by Ján Tomko 6 years, 11 months ago 
Add a new 'vsock' element for the vsock device.
The 'model' attribute is optional.
A <source cid> subelement should be used to specify the guest cid,
or <source auto='yes'/> should be used.

https://bugzilla.redhat.com/show_bug.cgi?id=1291851
---
 docs/formatdomain.html.in                     |  20 +++
 docs/schemas/domaincommon.rng                 |  29 ++++
 src/conf/domain_conf.c                        | 195 +++++++++++++++++++++++++-
 src/conf/domain_conf.h                        |  17 +++
 src/qemu/qemu_domain.c                        |   1 +
 src/qemu/qemu_domain_address.c                |  11 ++
 src/qemu/qemu_driver.c                        |   6 +
 src/qemu/qemu_hotplug.c                       |   1 +
 tests/qemuxml2argvdata/vhost-vsock-auto.xml   |  35 +++++
 tests/qemuxml2argvdata/vhost-vsock.xml        |  36 +++++
 tests/qemuxml2xmloutdata/vhost-vsock-auto.xml |  36 +++++
 tests/qemuxml2xmloutdata/vhost-vsock.xml      |   1 +
 tests/qemuxml2xmltest.c                       |   3 +
 13 files changed, 390 insertions(+), 1 deletion(-)
 create mode 100644 tests/qemuxml2argvdata/vhost-vsock-auto.xml
 create mode 100644 tests/qemuxml2argvdata/vhost-vsock.xml
 create mode 100644 tests/qemuxml2xmloutdata/vhost-vsock-auto.xml
 create mode 120000 tests/qemuxml2xmloutdata/vhost-vsock.xml

diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 0d0fd3b9f3..bfe7f757b8 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -8133,6 +8133,26 @@ qemu-kvm -net nic,model=? /dev/null
       </dd>
     </dl>
 
+    <h3><a id="vsock">Vsock</a></h3>
+
+    <p>A vsock host/guest interface. The <code>model</code> attribute
+    defaults to <code>virtio</code>.
+    The optional attribute <code>cid</code> of the source element
+    specifies the CID assigned to the guest. If the attribute
+    <code>auto</code> is set to <code>yes</code>, libvirt
+    will assign a free CID automatically on domain startup.
+    <span class="since">Since 4.4.0</span></p>
+
+<pre>
+...
+&lt;devices&gt;
+  &lt;vsock model='virtio'&gt;
+    &lt;source auto='no' cid='3'/&gt;
+  &lt;/vsock&gt;
+&lt;/devices&gt;
+...</pre>
+
+
     <h3><a id="seclabel">Security label</a></h3>
 
     <p>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 71ac3d079c..3ea5c91773 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -4140,6 +4140,32 @@
     </optional>
   </define>
 
+  <define name="vsock">
+    <element name="vsock">
+      <optional>
+        <attribute name="model">
+          <value>virtio</value>
+        </attribute>
+      </optional>
+      <interleave>
+        <element name="source">
+          <optional>
+            <attribute name="auto">
+              <ref name="virYesNo"/>
+            </attribute>
+          </optional>
+          <optional>
+            <attribute name="cid">
+              <ref name="unsignedInt"/>
+            </attribute>
+          </optional>
+        </element>
+        <optional>
+          <ref name="address"/>
+        </optional>
+      </interleave>
+    </element>
+  </define>
   <define name="iommu">
     <element name="iommu">
       <attribute name="model">
@@ -4687,6 +4713,9 @@
         <optional>
           <ref name="iommu"/>
         </optional>
+        <optional>
+          <ref name="vsock"/>
+        </optional>
       </interleave>
     </element>
   </define>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b2982fc3d4..1a3dbf884b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -256,7 +256,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
               "tpm",
               "panic",
               "memory",
-              "iommu")
+              "iommu",
+              "vsock")
 
 VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
               "none",
@@ -869,6 +870,10 @@ VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST,
 VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST,
               "intel")
 
+VIR_ENUM_IMPL(virDomainVsockModel, VIR_DOMAIN_VSOCK_MODEL_LAST,
+              "default",
+              "virtio")
+
 VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST,
               "default",
               "unmap",
@@ -2777,6 +2782,9 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
     case VIR_DOMAIN_DEVICE_IOMMU:
         VIR_FREE(def->data.iommu);
         break;
+    case VIR_DOMAIN_DEVICE_VSOCK:
+        virDomainVsockDefFree(def->data.vsock);
+        break;
     case VIR_DOMAIN_DEVICE_LAST:
     case VIR_DOMAIN_DEVICE_NONE:
         break;
@@ -3641,6 +3649,8 @@ virDomainDeviceGetInfo(virDomainDeviceDefPtr device)
         return &device->data.panic->info;
     case VIR_DOMAIN_DEVICE_MEMORY:
         return &device->data.memory->info;
+    case VIR_DOMAIN_DEVICE_VSOCK:
+        return &device->data.vsock->info;
 
     /* The following devices do not contain virDomainDeviceInfo */
     case VIR_DOMAIN_DEVICE_LEASE:
@@ -3838,6 +3848,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
             return rc;
     }
 
+    if (def->vsock) {
+        device.type = VIR_DOMAIN_DEVICE_VSOCK;
+        if ((rc = cb(def, &device, &def->vsock->info, opaque)) != 0)
+            return rc;
+    }
+
     /* Coverity is not very happy with this - all dead_error_condition */
 #if !STATIC_ANALYSIS
     /* This switch statement is here to trigger compiler warning when adding
@@ -3872,6 +3888,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
     case VIR_DOMAIN_DEVICE_RNG:
     case VIR_DOMAIN_DEVICE_MEMORY:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
         break;
     }
 #endif
@@ -5575,6 +5592,19 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem)
 }
 
 
+static int
+virDomainVsockDefValidate(const virDomainVsockDef *vsock)
+{
+    if (vsock->guest_cid > 0 && vsock->guest_cid <= 2) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("guest CIDs must be >= 3"));
+        return -1;
+    }
+
+    return 0;
+}
+
+
 static int
 virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
                                    const virDomainDef *def)
@@ -5610,6 +5640,9 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev,
     case VIR_DOMAIN_DEVICE_MEMORY:
         return virDomainMemoryDefValidate(dev->data.memory);
 
+    case VIR_DOMAIN_DEVICE_VSOCK:
+        return virDomainVsockDefValidate(dev->data.vsock);
+
     case VIR_DOMAIN_DEVICE_LEASE:
     case VIR_DOMAIN_DEVICE_FS:
     case VIR_DOMAIN_DEVICE_INPUT:
@@ -15878,6 +15911,68 @@ virDomainIOMMUDefParseXML(xmlNodePtr node,
 }
 
 
+static virDomainVsockDefPtr
+virDomainVsockDefParseXML(virDomainXMLOptionPtr xmlopt,
+                          xmlNodePtr node,
+                          xmlXPathContextPtr ctxt,
+                          unsigned int flags)
+{
+    virDomainVsockDefPtr vsock = NULL, ret = NULL;
+    xmlNodePtr save = ctxt->node;
+    xmlNodePtr source;
+    char *tmp = NULL;
+    int val;
+
+    ctxt->node = node;
+
+    if (!(vsock = virDomainVsockDefNew(xmlopt)))
+        goto cleanup;
+
+    if ((tmp = virXMLPropString(node, "model"))) {
+        if ((val = virDomainVsockModelTypeFromString(tmp)) < 0) {
+            virReportError(VIR_ERR_XML_ERROR, _("unknown vsock model: %s"), tmp);
+            goto cleanup;
+        }
+        vsock->model = val;
+    }
+
+    source = virXPathNode("./source", ctxt);
+
+    VIR_FREE(tmp);
+    if (source && (tmp = virXMLPropString(source, "cid"))) {
+        if (virStrToLong_uip(tmp, NULL, 10, &vsock->guest_cid) < 0 ||
+            vsock->guest_cid == 0) {
+            virReportError(VIR_ERR_XML_DETAIL,
+                           _("'cid' attribute must be a positive number: %s"),
+                           tmp);
+            goto cleanup;
+        }
+    }
+
+    VIR_FREE(tmp);
+    if (source && (tmp = virXMLPropString(source, "auto"))) {
+        val = virTristateBoolTypeFromString(tmp);
+        if (val < 0) {
+            virReportError(VIR_ERR_XML_DETAIL,
+                           _("'auto' attribute can be 'yes' or 'no': %s"),
+                           tmp);
+            goto cleanup;
+        }
+        vsock->auto_cid = val;
+    }
+
+    if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &vsock->info, flags) < 0)
+        goto cleanup;
+
+    VIR_STEAL_PTR(ret, vsock);
+
+ cleanup:
+    ctxt->node = save;
+    VIR_FREE(vsock);
+    VIR_FREE(tmp);
+    return ret;
+}
+
 virDomainDeviceDefPtr
 virDomainDeviceDefParse(const char *xmlStr,
                         const virDomainDef *def,
@@ -16033,6 +16128,11 @@ virDomainDeviceDefParse(const char *xmlStr,
         if (!(dev->data.iommu = virDomainIOMMUDefParseXML(node, ctxt)))
             goto error;
         break;
+    case VIR_DOMAIN_DEVICE_VSOCK:
+        if (!(dev->data.vsock = virDomainVsockDefParseXML(xmlopt, node, ctxt,
+                                                          flags)))
+            goto error;
+        break;
     case VIR_DOMAIN_DEVICE_NONE:
     case VIR_DOMAIN_DEVICE_LAST:
         break;
@@ -20435,6 +20535,22 @@ virDomainDefParseXML(xmlDocPtr xml,
     }
     VIR_FREE(nodes);
 
+    if ((n = virXPathNodeSet("./devices/vsock", ctxt, &nodes)) < 0)
+        goto error;
+
+    if (n > 1) {
+        virReportError(VIR_ERR_XML_ERROR, "%s",
+                       _("only a single vsock device is supported"));
+        goto error;
+    }
+
+    if (n > 0) {
+        if (!(def->vsock = virDomainVsockDefParseXML(xmlopt, nodes[0],
+                                                     ctxt, flags)))
+            goto error;
+    }
+    VIR_FREE(nodes);
+
     /* analysis of the user namespace mapping */
     if ((n = virXPathNodeSet("./idmap/uid", ctxt, &nodes)) < 0)
         goto error;
@@ -21996,6 +22112,26 @@ virDomainIOMMUDefCheckABIStability(virDomainIOMMUDefPtr src,
 }
 
 
+static bool
+virDomainVsockDefCheckABIStability(virDomainVsockDefPtr src,
+                                   virDomainVsockDefPtr dst)
+{
+    if (src->model != dst->model) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                       _("Target domain vsock device model '%s' "
+                         "does not match source '%s'"),
+                       virDomainVsockModelTypeToString(dst->model),
+                       virDomainVsockModelTypeToString(src->model));
+        return false;
+    }
+
+    if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info))
+        return false;
+
+    return true;
+}
+
+
 static bool
 virDomainDefVcpuCheckAbiStability(virDomainDefPtr src,
                                   virDomainDefPtr dst)
@@ -22441,6 +22577,17 @@ virDomainDefCheckABIStabilityFlags(virDomainDefPtr src,
         !virDomainIOMMUDefCheckABIStability(src->iommu, dst->iommu))
         goto error;
 
+    if (!!src->vsock != !!dst->vsock) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("Target domain vsock device count "
+                         "does not match source"));
+        goto error;
+    }
+
+    if (src->vsock &&
+        !virDomainVsockDefCheckABIStability(src->vsock, dst->vsock))
+        goto error;
+
     if (xmlopt && xmlopt->abi.domain &&
         !xmlopt->abi.domain(src, dst))
         goto error;
@@ -22478,6 +22625,7 @@ virDomainDefCheckABIStabilityFlags(virDomainDefPtr src,
     case VIR_DOMAIN_DEVICE_SHMEM:
     case VIR_DOMAIN_DEVICE_MEMORY:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
         break;
     }
 #endif
@@ -26735,6 +26883,46 @@ virDomainMemtuneFormat(virBufferPtr buf,
 }
 
 
+static int
+virDomainVsockDefFormat(virBufferPtr buf,
+                        virDomainVsockDefPtr vsock)
+{
+    virBuffer childBuf = VIR_BUFFER_INITIALIZER;
+    virBuffer attrBuf = VIR_BUFFER_INITIALIZER;
+    virBuffer sourceAttrBuf = VIR_BUFFER_INITIALIZER;
+    int ret = -1;
+
+    if (vsock->model) {
+        virBufferAsprintf(&attrBuf, " model='%s'",
+                          virDomainVsockModelTypeToString(vsock->model));
+    }
+
+    virBufferSetChildIndent(&childBuf, buf);
+
+    if (vsock->auto_cid != VIR_TRISTATE_BOOL_ABSENT) {
+        virBufferAsprintf(&sourceAttrBuf, " auto='%s'",
+                          virTristateBoolTypeToString(vsock->auto_cid));
+    }
+    if (vsock->guest_cid != 0)
+        virBufferAsprintf(&sourceAttrBuf, " cid='%u'", vsock->guest_cid);
+    if (virXMLFormatElement(&childBuf, "source", &sourceAttrBuf, NULL) < 0)
+        goto cleanup;
+
+    virDomainDeviceInfoFormat(&childBuf, &vsock->info, 0);
+
+    if (virXMLFormatElement(buf, "vsock", &attrBuf, &childBuf) < 0)
+        goto cleanup;
+
+    ret = 0;
+
+ cleanup:
+    virBufferFreeAndReset(&childBuf);
+    virBufferFreeAndReset(&attrBuf);
+    virBufferFreeAndReset(&sourceAttrBuf);
+    return ret;
+}
+
+
 /* This internal version appends to an existing buffer
  * (possibly with auto-indent), rather than flattening
  * to string.
@@ -27469,6 +27657,10 @@ virDomainDefFormatInternal(virDomainDefPtr def,
         virDomainIOMMUDefFormat(buf, def->iommu) < 0)
         goto error;
 
+    if (def->vsock &&
+        virDomainVsockDefFormat(buf, def->vsock) < 0)
+        goto error;
+
     virBufferAdjustIndent(buf, -2);
     virBufferAddLit(buf, "</devices>\n");
 
@@ -28595,6 +28787,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
     case VIR_DOMAIN_DEVICE_MEMBALLOON:
     case VIR_DOMAIN_DEVICE_NVRAM:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
     case VIR_DOMAIN_DEVICE_LAST:
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Copying definition of '%d' type "
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 4fa67ae7b7..a5e45bfda5 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -189,6 +189,7 @@ typedef enum {
     VIR_DOMAIN_DEVICE_PANIC,
     VIR_DOMAIN_DEVICE_MEMORY,
     VIR_DOMAIN_DEVICE_IOMMU,
+    VIR_DOMAIN_DEVICE_VSOCK,
 
     VIR_DOMAIN_DEVICE_LAST
 } virDomainDeviceType;
@@ -221,6 +222,7 @@ struct _virDomainDeviceDef {
         virDomainPanicDefPtr panic;
         virDomainMemoryDefPtr memory;
         virDomainIOMMUDefPtr iommu;
+        virDomainVsockDefPtr vsock;
     } data;
 };
 
@@ -2313,8 +2315,21 @@ struct _virDomainIOMMUDef {
     virTristateSwitch iotlb;
 };
 
+typedef enum {
+    VIR_DOMAIN_VSOCK_MODEL_DEFAULT,
+    VIR_DOMAIN_VSOCK_MODEL_VIRTIO,
+
+    VIR_DOMAIN_VSOCK_MODEL_LAST
+} virDomainVsockModel;
+
 struct _virDomainVsockDef {
     virObjectPtr privateData;
+
+    virDomainVsockModel model;
+    unsigned int guest_cid;
+    virTristateBool auto_cid;
+
+    virDomainDeviceInfo info;
 };
 
 struct _virDomainVirtioOptions {
@@ -2462,6 +2477,7 @@ struct _virDomainDef {
     virSysinfoDefPtr sysinfo;
     virDomainRedirFilterDefPtr redirfilter;
     virDomainIOMMUDefPtr iommu;
+    virDomainVsockDefPtr vsock;
 
     void *namespaceData;
     virDomainXMLNamespace ns;
@@ -3366,6 +3382,7 @@ VIR_ENUM_DECL(virDomainMemoryBackingModel)
 VIR_ENUM_DECL(virDomainMemorySource)
 VIR_ENUM_DECL(virDomainMemoryAllocation)
 VIR_ENUM_DECL(virDomainIOMMUModel)
+VIR_ENUM_DECL(virDomainVsockModel)
 VIR_ENUM_DECL(virDomainShmemModel)
 /* from libvirt.h */
 VIR_ENUM_DECL(virDomainState)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index d3beee5d87..6648933c80 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5132,6 +5132,7 @@ qemuDomainDeviceDefValidate(const virDomainDeviceDef *dev,
     case VIR_DOMAIN_DEVICE_TPM:
     case VIR_DOMAIN_DEVICE_PANIC:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
     case VIR_DOMAIN_DEVICE_NONE:
     case VIR_DOMAIN_DEVICE_LAST:
         break;
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index b7c82cb6f1..e9f460d77a 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -859,6 +859,9 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev,
         }
         break;
 
+    case VIR_DOMAIN_DEVICE_VSOCK:
+        return virtioFlags;
+
         /* These devices don't ever connect with PCI */
     case VIR_DOMAIN_DEVICE_NVRAM:
     case VIR_DOMAIN_DEVICE_TPM:
@@ -2152,6 +2155,14 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
         /* Nada - none are PCI based (yet) */
     }
 
+    if (def->vsock &&
+        virDeviceInfoPCIAddressWanted(&def->vsock->info)) {
+
+        if (qemuDomainPCIAddressReserveNextAddr(addrs,
+                                                &def->vsock->info) < 0)
+            goto error;
+    }
+
     return 0;
 
  error:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3a328e5d46..9d5cf4d61a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -7701,6 +7701,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
     case VIR_DOMAIN_DEVICE_TPM:
     case VIR_DOMAIN_DEVICE_PANIC:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
     case VIR_DOMAIN_DEVICE_LAST:
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("live attach of device '%s' is not supported"),
@@ -7800,6 +7801,7 @@ qemuDomainDetachDeviceLive(virDomainObjPtr vm,
     case VIR_DOMAIN_DEVICE_TPM:
     case VIR_DOMAIN_DEVICE_PANIC:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
     case VIR_DOMAIN_DEVICE_LAST:
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("live detach of device '%s' is not supported"),
@@ -7936,6 +7938,7 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
     case VIR_DOMAIN_DEVICE_TPM:
     case VIR_DOMAIN_DEVICE_PANIC:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
     case VIR_DOMAIN_DEVICE_LAST:
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("live update of device '%s' is not supported"),
@@ -8122,6 +8125,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef,
     case VIR_DOMAIN_DEVICE_TPM:
     case VIR_DOMAIN_DEVICE_PANIC:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
     case VIR_DOMAIN_DEVICE_LAST:
          virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                         _("persistent attach of device '%s' is not supported"),
@@ -8305,6 +8309,7 @@ qemuDomainDetachDeviceConfig(virDomainDefPtr vmdef,
     case VIR_DOMAIN_DEVICE_TPM:
     case VIR_DOMAIN_DEVICE_PANIC:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
     case VIR_DOMAIN_DEVICE_LAST:
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("persistent detach of device '%s' is not supported"),
@@ -8403,6 +8408,7 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
     case VIR_DOMAIN_DEVICE_TPM:
     case VIR_DOMAIN_DEVICE_PANIC:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
     case VIR_DOMAIN_DEVICE_LAST:
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("persistent update of device '%s' is not supported"),
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index b35594be5f..e429f54a43 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -4626,6 +4626,7 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
     case VIR_DOMAIN_DEVICE_TPM:
     case VIR_DOMAIN_DEVICE_PANIC:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
     case VIR_DOMAIN_DEVICE_LAST:
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
                        _("don't know how to remove a %s device"),
diff --git a/tests/qemuxml2argvdata/vhost-vsock-auto.xml b/tests/qemuxml2argvdata/vhost-vsock-auto.xml
new file mode 100644
index 0000000000..729d58bea4
--- /dev/null
+++ b/tests/qemuxml2argvdata/vhost-vsock-auto.xml
@@ -0,0 +1,35 @@
+<domain type='qemu'>
+  <name>test</name>
+  <uuid>bba65c0e-c049-934f-b6aa-4e2c0582acdf</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-0.13'>hvm</type>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='virtio-serial' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+    <vsock>
+      <source auto='yes'/>
+    </vsock>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvdata/vhost-vsock.xml b/tests/qemuxml2argvdata/vhost-vsock.xml
new file mode 100644
index 0000000000..ac210cb116
--- /dev/null
+++ b/tests/qemuxml2argvdata/vhost-vsock.xml
@@ -0,0 +1,36 @@
+<domain type='qemu'>
+  <name>test</name>
+  <uuid>bba65c0e-c049-934f-b6aa-4e2c0582acdf</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-0.13'>hvm</type>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='virtio-serial' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+    <vsock model='virtio'>
+      <source cid='4'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0'/>
+    </vsock>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/vhost-vsock-auto.xml b/tests/qemuxml2xmloutdata/vhost-vsock-auto.xml
new file mode 100644
index 0000000000..65f18b96d9
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/vhost-vsock-auto.xml
@@ -0,0 +1,36 @@
+<domain type='qemu'>
+  <name>test</name>
+  <uuid>bba65c0e-c049-934f-b6aa-4e2c0582acdf</uuid>
+  <memory unit='KiB'>1048576</memory>
+  <currentMemory unit='KiB'>1048576</currentMemory>
+  <vcpu placement='static'>1</vcpu>
+  <os>
+    <type arch='x86_64' machine='pc-0.13'>hvm</type>
+    <boot dev='hd'/>
+    <bootmenu enable='yes'/>
+  </os>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>restart</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu-system-x86_64</emulator>
+    <controller type='usb' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
+    </controller>
+    <controller type='virtio-serial' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
+    </controller>
+    <controller type='ide' index='0'>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
+    </controller>
+    <controller type='pci' index='0' model='pci-root'/>
+    <input type='mouse' bus='ps2'/>
+    <input type='keyboard' bus='ps2'/>
+    <memballoon model='none'/>
+    <vsock>
+      <source auto='yes'/>
+      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
+    </vsock>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2xmloutdata/vhost-vsock.xml b/tests/qemuxml2xmloutdata/vhost-vsock.xml
new file mode 120000
index 0000000000..bb24241fb2
--- /dev/null
+++ b/tests/qemuxml2xmloutdata/vhost-vsock.xml
@@ -0,0 +1 @@
+../qemuxml2argvdata/vhost-vsock.xml
\ No newline at end of file
diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
index 7cedc2b999..cd6538a442 100644
--- a/tests/qemuxml2xmltest.c
+++ b/tests/qemuxml2xmltest.c
@@ -1209,6 +1209,9 @@ mymain(void)
     DO_TEST_STATUS("migration-in-params");
     DO_TEST_STATUS("migration-out-params");
 
+    DO_TEST("vhost-vsock", NONE);
+    DO_TEST("vhost-vsock-auto", NONE);
+
     if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
         virFileDeleteTree(fakerootdir);
 
-- 
2.16.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/7] conf: introduce <vsock> element
Posted by Peter Krempa 6 years, 11 months ago 
On Thu, May 24, 2018 at 12:39:11 +0200, Ján Tomko wrote:
> Add a new 'vsock' element for the vsock device.
> The 'model' attribute is optional.
> A <source cid> subelement should be used to specify the guest cid,
> or <source auto='yes'/> should be used.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1291851
> ---
>  docs/formatdomain.html.in                     |  20 +++
>  docs/schemas/domaincommon.rng                 |  29 ++++
>  src/conf/domain_conf.c                        | 195 +++++++++++++++++++++++++-
>  src/conf/domain_conf.h                        |  17 +++
>  src/qemu/qemu_domain.c                        |   1 +
>  src/qemu/qemu_domain_address.c                |  11 ++
>  src/qemu/qemu_driver.c                        |   6 +
>  src/qemu/qemu_hotplug.c                       |   1 +
>  tests/qemuxml2argvdata/vhost-vsock-auto.xml   |  35 +++++
>  tests/qemuxml2argvdata/vhost-vsock.xml        |  36 +++++
>  tests/qemuxml2xmloutdata/vhost-vsock-auto.xml |  36 +++++
>  tests/qemuxml2xmloutdata/vhost-vsock.xml      |   1 +
>  tests/qemuxml2xmltest.c                       |   3 +
>  13 files changed, 390 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/vhost-vsock-auto.xml
>  create mode 100644 tests/qemuxml2argvdata/vhost-vsock.xml
>  create mode 100644 tests/qemuxml2xmloutdata/vhost-vsock-auto.xml
>  create mode 120000 tests/qemuxml2xmloutdata/vhost-vsock.xml

I've recently added a new swithc case which breaks with the following
message:

qemu/qemu_domain.c: In function 'qemuDomainDeviceDefPostParse':
qemu/qemu_domain.c:5802:5: error: enumeration value 'VIR_DOMAIN_DEVICE_VSOCK' not handled in switch [-Werror=switch-enum]
     switch ((virDomainDeviceType) dev->type) {
     ^~~~~~

Apply the following patch:

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 527781c0fa..3e8e6358de 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -5845,6 +5845,7 @@ qemuDomainDeviceDefPostParse(virDomainDeviceDefPtr dev,
     case VIR_DOMAIN_DEVICE_TPM:
     case VIR_DOMAIN_DEVICE_MEMORY:
     case VIR_DOMAIN_DEVICE_IOMMU:
+    case VIR_DOMAIN_DEVICE_VSOCK:
         ret = 0;
         break;



> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 0d0fd3b9f3..bfe7f757b8 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -8133,6 +8133,26 @@ qemu-kvm -net nic,model=? /dev/null
>        </dd>
>      </dl>
>  
> +    <h3><a id="vsock">Vsock</a></h3>
> +
> +    <p>A vsock host/guest interface. The <code>model</code> attribute
> +    defaults to <code>virtio</code>.
> +    The optional attribute <code>cid</code> of the source element
> +    specifies the CID assigned to the guest. If the attribute
> +    <code>auto</code> is set to <code>yes</code>, libvirt
> +    will assign a free CID automatically on domain startup.
> +    <span class="since">Since 4.4.0</span></p>

Should we perhaps mention some docs where users can figure out how to
use it?

Also please mention what the default value of 'auto' is. e.g. what the
following definition actually configures:

<vsock model='virtio'/>

> +
> +<pre>
> +...
> +&lt;devices&gt;
> +  &lt;vsock model='virtio'&gt;
> +    &lt;source auto='no' cid='3'/&gt;
> +  &lt;/vsock&gt;
> +&lt;/devices&gt;
> +...</pre>
> +
> +
>      <h3><a id="seclabel">Security label</a></h3>
>  
>      <p>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 71ac3d079c..3ea5c91773 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -4140,6 +4140,32 @@
>      </optional>
>    </define>
>  
> +  <define name="vsock">
> +    <element name="vsock">
> +      <optional>
> +        <attribute name="model">
> +          <value>virtio</value>
> +        </attribute>
> +      </optional>
> +      <interleave>
> +        <element name="source">

So, source is mandatory? It should be noted in the docs ...

> +          <optional>
> +            <attribute name="auto">
> +              <ref name="virYesNo"/>
> +            </attribute>
> +          </optional>
> +          <optional>
> +            <attribute name="cid">
> +              <ref name="unsignedInt"/>
> +            </attribute>
> +          </optional>

Also both attributes are optional which is kind of weird if <source> is
mandatory.

> +        </element>
> +        <optional>
> +          <ref name="address"/>
> +        </optional>
> +      </interleave>
> +    </element>
> +  </define>

[...]

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b2982fc3d4..1a3dbf884b 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

[...]

> @@ -15878,6 +15911,68 @@ virDomainIOMMUDefParseXML(xmlNodePtr node,
>  }
>  
>  
> +static virDomainVsockDefPtr
> +virDomainVsockDefParseXML(virDomainXMLOptionPtr xmlopt,
> +                          xmlNodePtr node,
> +                          xmlXPathContextPtr ctxt,
> +                          unsigned int flags)
> +{
> +    virDomainVsockDefPtr vsock = NULL, ret = NULL;
> +    xmlNodePtr save = ctxt->node;
> +    xmlNodePtr source;
> +    char *tmp = NULL;
> +    int val;
> +
> +    ctxt->node = node;
> +
> +    if (!(vsock = virDomainVsockDefNew(xmlopt)))
> +        goto cleanup;
> +
> +    if ((tmp = virXMLPropString(node, "model"))) {
> +        if ((val = virDomainVsockModelTypeFromString(tmp)) < 0) {

Model 'default' should not be allowed.

> +            virReportError(VIR_ERR_XML_ERROR, _("unknown vsock model: %s"), tmp);
> +            goto cleanup;
> +        }
> +        vsock->model = val;
> +    }
> +
> +    source = virXPathNode("./source", ctxt);
> +
> +    VIR_FREE(tmp);
> +    if (source && (tmp = virXMLPropString(source, "cid"))) {

'source' is the common condition in both if the statements below, so
perhaps it can be extracted one level up.

> +        if (virStrToLong_uip(tmp, NULL, 10, &vsock->guest_cid) < 0 ||
> +            vsock->guest_cid == 0) {
> +            virReportError(VIR_ERR_XML_DETAIL,
> +                           _("'cid' attribute must be a positive number: %s"),
> +                           tmp);
> +            goto cleanup;
> +        }
> +    }
> +
> +    VIR_FREE(tmp);
> +    if (source && (tmp = virXMLPropString(source, "auto"))) {
> +        val = virTristateBoolTypeFromString(tmp);
> +        if (val < 0) {

the "default" value should not be allowed ...

> +            virReportError(VIR_ERR_XML_DETAIL,
> +                           _("'auto' attribute can be 'yes' or 'no': %s"),

... specifically given this error message.

> +                           tmp);
> +            goto cleanup;
> +        }
> +        vsock->auto_cid = val;
> +    }
> +
> +    if (virDomainDeviceInfoParseXML(xmlopt, node, NULL, &vsock->info, flags) < 0)
> +        goto cleanup;
> +
> +    VIR_STEAL_PTR(ret, vsock);
> +
> + cleanup:
> +    ctxt->node = save;
> +    VIR_FREE(vsock);
> +    VIR_FREE(tmp);
> +    return ret;
> +}


[...]

> @@ -28595,6 +28787,7 @@ virDomainDeviceDefCopy(virDomainDeviceDefPtr src,
>      case VIR_DOMAIN_DEVICE_MEMBALLOON:
>      case VIR_DOMAIN_DEVICE_NVRAM:
>      case VIR_DOMAIN_DEVICE_IOMMU:
> +    case VIR_DOMAIN_DEVICE_VSOCK:

This should be very easy to implement.


[...]

> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index b35594be5f..e429f54a43 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -4626,6 +4626,7 @@ qemuDomainRemoveDevice(virQEMUDriverPtr driver,
>      case VIR_DOMAIN_DEVICE_TPM:
>      case VIR_DOMAIN_DEVICE_PANIC:
>      case VIR_DOMAIN_DEVICE_IOMMU:
> +    case VIR_DOMAIN_DEVICE_VSOCK:

Note that I've did not look through the impl patch yet, so the below
comment may be actually moot.

At least this should be implemented as you can trigger a device detach
from the guest as well. Since this does not have any backend data, the
removal function is trivial.

>      case VIR_DOMAIN_DEVICE_LAST:
>          virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
>                         _("don't know how to remove a %s device"),
> diff --git a/tests/qemuxml2argvdata/vhost-vsock-auto.xml b/tests/qemuxml2argvdata/vhost-vsock-auto.xml
> new file mode 100644
> index 0000000000..729d58bea4
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/vhost-vsock-auto.xml
> @@ -0,0 +1,35 @@
> +<domain type='qemu'>
> +  <name>test</name>
> +  <uuid>bba65c0e-c049-934f-b6aa-4e2c0582acdf</uuid>
> +  <memory unit='KiB'>1048576</memory>
> +  <currentMemory unit='KiB'>1048576</currentMemory>
> +  <vcpu placement='static'>1</vcpu>
> +  <os>
> +    <type arch='x86_64' machine='pc-0.13'>hvm</type>

Our XMLs are living in the past.

> +    <boot dev='hd'/>
> +    <bootmenu enable='yes'/>

For an ACK please state how you plan to deal with the schema issues I've
pointed out. All other necessary changes were pointed out.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/7] conf: introduce <vsock> element
Posted by Ján Tomko 6 years, 11 months ago 
On Tue, May 29, 2018 at 10:26:20AM +0200, Peter Krempa wrote:
>On Thu, May 24, 2018 at 12:39:11 +0200, Ján Tomko wrote:
>> Add a new 'vsock' element for the vsock device.
>> The 'model' attribute is optional.
>> A <source cid> subelement should be used to specify the guest cid,
>> or <source auto='yes'/> should be used.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1291851
>> ---
>>  docs/formatdomain.html.in                     |  20 +++
>>  docs/schemas/domaincommon.rng                 |  29 ++++
>>  src/conf/domain_conf.c                        | 195 +++++++++++++++++++++++++-
>>  src/conf/domain_conf.h                        |  17 +++
>>  src/qemu/qemu_domain.c                        |   1 +
>>  src/qemu/qemu_domain_address.c                |  11 ++
>>  src/qemu/qemu_driver.c                        |   6 +
>>  src/qemu/qemu_hotplug.c                       |   1 +
>>  tests/qemuxml2argvdata/vhost-vsock-auto.xml   |  35 +++++
>>  tests/qemuxml2argvdata/vhost-vsock.xml        |  36 +++++
>>  tests/qemuxml2xmloutdata/vhost-vsock-auto.xml |  36 +++++
>>  tests/qemuxml2xmloutdata/vhost-vsock.xml      |   1 +
>>  tests/qemuxml2xmltest.c                       |   3 +
>>  13 files changed, 390 insertions(+), 1 deletion(-)
>>  create mode 100644 tests/qemuxml2argvdata/vhost-vsock-auto.xml
>>  create mode 100644 tests/qemuxml2argvdata/vhost-vsock.xml
>>  create mode 100644 tests/qemuxml2xmloutdata/vhost-vsock-auto.xml
>>  create mode 120000 tests/qemuxml2xmloutdata/vhost-vsock.xml
>

>> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
>> index 71ac3d079c..3ea5c91773 100644
>> --- a/docs/schemas/domaincommon.rng
>> +++ b/docs/schemas/domaincommon.rng
>> @@ -4140,6 +4140,32 @@
>>      </optional>
>>    </define>
>>
>> +  <define name="vsock">
>> +    <element name="vsock">
>> +      <optional>
>> +        <attribute name="model">
>> +          <value>virtio</value>
>> +        </attribute>
>> +      </optional>
>> +      <interleave>
>> +        <element name="source">
>
>So, source is mandatory? It should be noted in the docs ...
>
>> +          <optional>
>> +            <attribute name="auto">
>> +              <ref name="virYesNo"/>
>> +            </attribute>
>> +          </optional>
>> +          <optional>
>> +            <attribute name="cid">
>> +              <ref name="unsignedInt"/>
>> +            </attribute>
>> +          </optional>
>
>Also both attributes are optional which is kind of weird if <source> is
>mandatory.
>
>> +        </element>
>> +        <optional>
>> +          <ref name="address"/>
>> +        </optional>
>> +      </interleave>
>> +    </element>
>> +  </define>
>
>[...]
>

[...]

>> diff --git a/tests/qemuxml2argvdata/vhost-vsock-auto.xml b/tests/qemuxml2argvdata/vhost-vsock-auto.xml
>> new file mode 100644
>> index 0000000000..729d58bea4
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/vhost-vsock-auto.xml
>> @@ -0,0 +1,35 @@
>> +<domain type='qemu'>
>> +  <name>test</name>
>> +  <uuid>bba65c0e-c049-934f-b6aa-4e2c0582acdf</uuid>
>> +  <memory unit='KiB'>1048576</memory>
>> +  <currentMemory unit='KiB'>1048576</currentMemory>
>> +  <vcpu placement='static'>1</vcpu>
>> +  <os>
>> +    <type arch='x86_64' machine='pc-0.13'>hvm</type>
>
>Our XMLs are living in the past.
>
>> +    <boot dev='hd'/>
>> +    <bootmenu enable='yes'/>
>
>For an ACK please state how you plan to deal with the schema issues I've
>pointed out. All other necessary changes were pointed out.

The idea is to transform <vsock> -> <vsock model='virtio'/> in the QEMU
driver.

<source> and both of its attributes will be optional in the schema
and in case 'auto' is missing, we'll tune it up in PostParse:

    if (vsock->auto_cid == VIR_TRISTATE_BOOL_ABSENT) {
        if (vsock->guest_cid != 0)
            vsock->auto_cid = VIR_TRISTATE_BOOL_NO;
        else
            vsock->auto_cid = VIR_TRISTATE_BOOL_YES;
    }

Jano



--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/7] conf: introduce <vsock> element
Posted by Peter Krempa 6 years, 11 months ago 
On Tue, May 29, 2018 at 13:36:51 +0200, Ján Tomko wrote:
> On Tue, May 29, 2018 at 10:26:20AM +0200, Peter Krempa wrote:
> > On Thu, May 24, 2018 at 12:39:11 +0200, Ján Tomko wrote:
> > > Add a new 'vsock' element for the vsock device.
> > > The 'model' attribute is optional.
> > > A <source cid> subelement should be used to specify the guest cid,
> > > or <source auto='yes'/> should be used.
> > > 
> > > https://bugzilla.redhat.com/show_bug.cgi?id=1291851
> > > ---
> > >  docs/formatdomain.html.in                     |  20 +++
> > >  docs/schemas/domaincommon.rng                 |  29 ++++
> > >  src/conf/domain_conf.c                        | 195 +++++++++++++++++++++++++-
> > >  src/conf/domain_conf.h                        |  17 +++
> > >  src/qemu/qemu_domain.c                        |   1 +
> > >  src/qemu/qemu_domain_address.c                |  11 ++
> > >  src/qemu/qemu_driver.c                        |   6 +
> > >  src/qemu/qemu_hotplug.c                       |   1 +
> > >  tests/qemuxml2argvdata/vhost-vsock-auto.xml   |  35 +++++
> > >  tests/qemuxml2argvdata/vhost-vsock.xml        |  36 +++++
> > >  tests/qemuxml2xmloutdata/vhost-vsock-auto.xml |  36 +++++
> > >  tests/qemuxml2xmloutdata/vhost-vsock.xml      |   1 +
> > >  tests/qemuxml2xmltest.c                       |   3 +
> > >  13 files changed, 390 insertions(+), 1 deletion(-)
> > >  create mode 100644 tests/qemuxml2argvdata/vhost-vsock-auto.xml
> > >  create mode 100644 tests/qemuxml2argvdata/vhost-vsock.xml
> > >  create mode 100644 tests/qemuxml2xmloutdata/vhost-vsock-auto.xml
> > >  create mode 120000 tests/qemuxml2xmloutdata/vhost-vsock.xml
> > 
> 
> > > diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> > > index 71ac3d079c..3ea5c91773 100644
> > > --- a/docs/schemas/domaincommon.rng
> > > +++ b/docs/schemas/domaincommon.rng
> > > @@ -4140,6 +4140,32 @@
> > >      </optional>
> > >    </define>
> > > 
> > > +  <define name="vsock">
> > > +    <element name="vsock">
> > > +      <optional>
> > > +        <attribute name="model">
> > > +          <value>virtio</value>
> > > +        </attribute>
> > > +      </optional>
> > > +      <interleave>
> > > +        <element name="source">
> > 
> > So, source is mandatory? It should be noted in the docs ...
> > 
> > > +          <optional>
> > > +            <attribute name="auto">
> > > +              <ref name="virYesNo"/>
> > > +            </attribute>
> > > +          </optional>
> > > +          <optional>
> > > +            <attribute name="cid">
> > > +              <ref name="unsignedInt"/>
> > > +            </attribute>
> > > +          </optional>
> > 
> > Also both attributes are optional which is kind of weird if <source> is
> > mandatory.
> > 
> > > +        </element>
> > > +        <optional>
> > > +          <ref name="address"/>
> > > +        </optional>
> > > +      </interleave>
> > > +    </element>
> > > +  </define>
> > 
> > [...]
> > 
> 
> [...]
> 
> > > diff --git a/tests/qemuxml2argvdata/vhost-vsock-auto.xml b/tests/qemuxml2argvdata/vhost-vsock-auto.xml
> > > new file mode 100644
> > > index 0000000000..729d58bea4
> > > --- /dev/null
> > > +++ b/tests/qemuxml2argvdata/vhost-vsock-auto.xml
> > > @@ -0,0 +1,35 @@
> > > +<domain type='qemu'>
> > > +  <name>test</name>
> > > +  <uuid>bba65c0e-c049-934f-b6aa-4e2c0582acdf</uuid>
> > > +  <memory unit='KiB'>1048576</memory>
> > > +  <currentMemory unit='KiB'>1048576</currentMemory>
> > > +  <vcpu placement='static'>1</vcpu>
> > > +  <os>
> > > +    <type arch='x86_64' machine='pc-0.13'>hvm</type>
> > 
> > Our XMLs are living in the past.
> > 
> > > +    <boot dev='hd'/>
> > > +    <bootmenu enable='yes'/>
> > 
> > For an ACK please state how you plan to deal with the schema issues I've
> > pointed out. All other necessary changes were pointed out.
> 
> The idea is to transform <vsock> -> <vsock model='virtio'/> in the QEMU
> driver.
> 
> <source> and both of its attributes will be optional in the schema
> and in case 'auto' is missing, we'll tune it up in PostParse:
> 
>    if (vsock->auto_cid == VIR_TRISTATE_BOOL_ABSENT) {
>        if (vsock->guest_cid != 0)
>            vsock->auto_cid = VIR_TRISTATE_BOOL_NO;
>        else
>            vsock->auto_cid = VIR_TRISTATE_BOOL_YES;
>    }

Okay. ACK if you make <source> optional and add this code.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/7] conf: introduce <vsock> element
Posted by Daniel P. Berrangé 6 years, 11 months ago 
On Thu, May 24, 2018 at 12:39:11PM +0200, Ján Tomko wrote:
> Add a new 'vsock' element for the vsock device.
> The 'model' attribute is optional.
> A <source cid> subelement should be used to specify the guest cid,
> or <source auto='yes'/> should be used.

I always find the  <source> vs <target> naming somewhat confusing
because we are not consistent in how we use them.

How about just avoiding the term entirely eg similar to how we declare
MAC with <mac address="...">:

   <cid address="3"/>

optionally having auto=yes|no


Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Patchew 2.1-devel-8c64711

© 2016 - 2025 Red Hat, Inc.