[libvirt] [PATCH v3 07/10] conf: Wire up the vhost-scsi connection from/to XML

John Ferlan jferlan at redhat.com
Fri Nov 11 21:53:34 UTC 2016


need a commit message here.

On 11/08/2016 01:26 PM, Eric Farman wrote:
> Signed-off-by: Eric Farman <farman at linux.vnet.ibm.com>
> ---
>  docs/schemas/domaincommon.rng | 23 ++++++++++++
>  src/conf/domain_audit.c       |  7 ++++
>  src/conf/domain_conf.c        | 81 +++++++++++++++++++++++++++++++++++++++++--
>  3 files changed, 109 insertions(+), 2 deletions(-)
> 

Beyond the "Host" to "SCSIHost" type changes...

Since this is where the device is being added - this is when the address
adjustment functions should be addressed. I got really hung up to day
trying to look for examples - hopefully I didn't make too much of a mess...

First off, the virDomainHostdevDefParseXML should be adjusted to ensure
that if the user does provide an address - that it is valid for the
device.  IOW: Follow the SUBSYS_TYPE_PCI: case more or less to ensure
the def->info->type if not NONE is either TYPE_PCI or TYPE_CCW.

The virDomainDeviceDefPostParseInternal post processing code should
handle setting an address if a valid one (checked earlier) isn't
supplied. This would save the address in the domain/config XML. For PCI
it would be qemuDomainAssignDevicePCISlots... I'm not clear if doing the
same for CCW is ever done.

In any case, qemuDomainAssignDevicePCISlots does go through the hostdev
list and reserves PCI address for PCI hostdevs, which I believe would
also work for this would be. Again, I'm not clear if/why CCW would be
handled.

NB: I haven't gone and looked for every new subsys case to ensure that
things were addressed for the case's type - just ran out of time and
energy.  But that is something that should be done by the end of patch8
(which I assume now gets merged into here eventually).

> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 19d45fd..bb903ef 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -3974,6 +3974,7 @@
>        <ref name="hostdevsubsyspci"/>
>        <ref name="hostdevsubsysusb"/>
>        <ref name="hostdevsubsysscsi"/>
> +      <ref name="hostdevsubsyshost"/>
>      </choice>
>    </define>
>  
> @@ -4102,6 +4103,28 @@
>      </element>
>    </define>
>  
> +  <define name="hostdevsubsyshost">
> +    <attribute name="type">
> +      <value>scsi_host</value>
> +    </attribute>
> +    <element name="source">
> +      <choice>
> +        <group>
> +          <attribute name="protocol">
> +            <choice>
> +              <value>vhost</value>     <!-- vhost, required -->
> +            </choice>
> +          </attribute>
> +          <attribute name="wwpn">
> +            <data type="string">
> +              <param name="pattern">(naa\.)[0-9a-fA-F]{16}</param>
> +            </data>
> +          </attribute>
> +        </group>
> +      </choice>
> +    </element>
> +  </define>
> +
>    <define name="hostdevcapsstorage">
>      <attribute name="type">
>        <value>storage</value>
> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c
> index 2decf02..844b3cd 100644
> --- a/src/conf/domain_audit.c
> +++ b/src/conf/domain_audit.c
> @@ -392,6 +392,7 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev,
>      virDomainHostdevSubsysUSBPtr usbsrc = &hostdev->source.subsys.u.usb;
>      virDomainHostdevSubsysPCIPtr pcisrc = &hostdev->source.subsys.u.pci;
>      virDomainHostdevSubsysSCSIPtr scsisrc = &hostdev->source.subsys.u.scsi;
> +    virDomainHostdevSubsysHostPtr hostsrc = &hostdev->source.subsys.u.host;
>  
>      virUUIDFormat(vm->def->uuid, uuidstr);
>      if (!(vmname = virAuditEncode("vm", vm->def->name))) {
> @@ -444,6 +445,12 @@ virDomainAuditHostdev(virDomainObjPtr vm, virDomainHostdevDefPtr hostdev,
>              }
>              break;
>          }
> +        case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
> +            if (VIR_STRDUP_QUIET(address, hostsrc->wwpn) < 0) {
> +                VIR_WARN("OOM while encoding audit message");
> +                goto cleanup;
> +            }
> +            break;
>          default:
>              VIR_WARN("Unexpected hostdev type while encoding audit message: %d",
>                       hostdev->source.subsys.type);
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b8a3366..75cacd9 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -2323,6 +2323,9 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
>              } else {
>                  VIR_FREE(scsisrc->u.host.adapter);
>              }
> +        } else if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST) {
> +            virDomainHostdevSubsysHostPtr hostsrc = &def->source.subsys.u.host;
> +            VIR_FREE(hostsrc->wwpn);
>          }
>          break;
>      }
> @@ -6092,6 +6095,55 @@ virDomainHostdevSubsysSCSIDefParseXML(xmlNodePtr sourcenode,
>      return ret;
>  }
>  
> +static int
> +virDomainHostdevSubsysHostDefParseXML(xmlNodePtr sourcenode,
> +                                      virDomainHostdevDefPtr def)
> +{
> +    char *protocol = NULL;
> +    virDomainHostdevSubsysHostPtr hostsrc = &def->source.subsys.u.host;
> +
> +    if ((protocol = virXMLPropString(sourcenode, "protocol"))) {
> +        hostsrc->protocol =
> +            virDomainHostdevSubsysHostProtocolTypeFromString(protocol);
> +        if (hostsrc->protocol <= 0) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("Unknown scsi_host subsystem protocol '%s'"),
> +                           protocol);
> +            goto cleanup;
> +        }
> +    }

Since protocol is required the logic is :

    if (!(protocol = virXMLPropString(sourcenode, "protocol")))
        virReportError(... _("Missing vhost-scsi 'protocol' attribute'"));
        return -1;
    }

    if ((hostsrc->protocol =
         virDomainHostdevSubsysHostProtocolTypeFromString(protocol)) <= 0) {
        virReportError(...);
        goto cleanup;
    }

> +
> +    switch ((virDomainHostdevSubsysHostProtocolType) hostsrc->protocol) {
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_VHOST:
> +        if (!(hostsrc->wwpn = virXMLPropString(sourcenode, "wwpn"))) {
> +            virReportError(VIR_ERR_XML_ERROR, "%s",
> +                           _("missing vhost-scsi hostdev source path name"));

Technically it's missing the required 'wwpn' value

> +            goto cleanup;
> +        }
> +
> +        if (!STRPREFIX(hostsrc->wwpn, "naa.") ||
> +            strlen(hostsrc->wwpn) != strlen("naa.") + 16) {

I assume the wwpn minus the naa. needs to be valid - see virValidateWWN
and just pass wwpn + 4

> +            virReportError(VIR_ERR_XML_ERROR, "%s", _("malformed 'wwpn' value"));
> +            goto cleanup;
> +        }
> +        break;
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_NONE:
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_HOST_PROTOCOL_TYPE_LAST:
> +        virReportError(VIR_ERR_XML_ERROR,
> +                       _("Invalid hostdev protocol '%s'"),
> +                       virDomainHostdevSubsysHostProtocolTypeToString(def->source.subsys.type));

Use hostsrc->protocol for the argument

John

> +        goto cleanup;
> +        break;
> +    }
> +
> +    return 0;
> +
> + cleanup:
> +    VIR_FREE(hostsrc->wwpn);
> +    VIR_FREE(protocol);
> +    return -1;
> +}
> +
>  
>  static int
>  virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
> @@ -6216,6 +6268,11 @@ virDomainHostdevDefParseXMLSubsys(xmlNodePtr node,
>              goto error;
>          break;
>  
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
> +        if (virDomainHostdevSubsysHostDefParseXML(sourcenode, def) < 0)
> +            goto error;
> +        break;
> +
>      default:
>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                         _("address type='%s' not supported in hostdev interfaces"),
> @@ -13908,7 +13965,13 @@ virDomainHostdevMatchSubsys(virDomainHostdevDefPtr a,
>          else
>              return virDomainHostdevMatchSubsysSCSIHost(a, b);
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
> -        /* Fall through for now */
> +        if (a->source.subsys.u.host.protocol !=
> +            b->source.subsys.u.host.protocol)
> +            return 0;
> +        if (STREQ(a->source.subsys.u.host.wwpn, b->source.subsys.u.host.wwpn))
> +            return 1;
> +        else
> +            return 0;
>      case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST:
>          return 0;
>      }
> @@ -20815,9 +20878,11 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
>                                  unsigned int flags,
>                                  bool includeTypeInAddr)
>  {
> +    bool closedSource = false;
>      virDomainHostdevSubsysUSBPtr usbsrc = &def->source.subsys.u.usb;
>      virDomainHostdevSubsysPCIPtr pcisrc = &def->source.subsys.u.pci;
>      virDomainHostdevSubsysSCSIPtr scsisrc = &def->source.subsys.u.scsi;
> +    virDomainHostdevSubsysHostPtr hostsrc = &def->source.subsys.u.host;
>      virDomainHostdevSubsysSCSIHostPtr scsihostsrc = &scsisrc->u.host;
>      virDomainHostdevSubsysSCSIiSCSIPtr iscsisrc = &scsisrc->u.iscsi;
>  
> @@ -20858,6 +20923,15 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
>                            protocol, iscsisrc->path);
>      }
>  
> +    if (def->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST) {
> +        const char *protocol =
> +            virDomainHostdevSubsysHostProtocolTypeToString(hostsrc->protocol);
> +        closedSource = true;
> +
> +        virBufferAsprintf(buf, " protocol='%s' wwpn='%s'/",
> +                          protocol, hostsrc->wwpn);
> +    }
> +
>      virBufferAddLit(buf, ">\n");
>  
>      virBufferAdjustIndent(buf, 2);
> @@ -20911,6 +20985,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
>                                scsihostsrc->unit);
>          }
>          break;
> +    case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_HOST:
> +        break;
>      default:
>          virReportError(VIR_ERR_INTERNAL_ERROR,
>                         _("unexpected hostdev type %d"),
> @@ -20926,7 +21002,8 @@ virDomainHostdevDefFormatSubsys(virBufferPtr buf,
>      }
>  
>      virBufferAdjustIndent(buf, -2);
> -    virBufferAddLit(buf, "</source>\n");
> +    if (!closedSource)
> +        virBufferAddLit(buf, "</source>\n");
>  
>      return 0;
>  }
> 




More information about the libvir-list mailing list