[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 6/6] conf: parse/format type='hostdev' network interfaces



On 02/20/2012 10:10 AM, Laine Stump wrote:
> This is the new interface type that sets up a PCI/USB network device
> to be assigned to the guest with PCI/USB passthrough after
> initializing some network device-specific things from the config
> (e.g. MAC address, virtualport profile parameters). Here is an example
> of the syntax:
> 
>   <interface type='hostdev' managed='yes'>
>     <source>
>       <address type='pci' domain='0' bus='0' slot='4' function='0'/>
>     </source>
>     <mac address='00:11:22:33:44:55'/>
>     <address type='pci' domain='0' bus='0' slot='7' function='0'/>
>   </interface>
> 
> This would assign the PCI card from bus 0 slot 4 function 0 on the
> host, to bus 0 slot 7 function 0 on the guest, but would first set the
> MAC address of the card to 00:11:22:33:44:55.
> 
> Although it's not expected to be used very much, usb network hostdevs
> are also supported for completeness.

Even for common things like USB wifi sticks?  (Hmm, I've got one of
those lying around - might be fun to play with :)

> @@ -966,6 +967,12 @@ virDomainActualNetDefFree(virDomainActualNetDefPtr def)
>          VIR_FREE(def->data.direct.linkdev);
>          VIR_FREE(def->data.direct.virtPortProfile);
>          break;
> +    case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> +        /* currently there is nothing in a virDomainHostdevDef
> +         * that requires freeing.
> +         */
> +        VIR_FREE(def->data.hostdev.virtPortProfile);

Is that comment still accurate? (Two instances)

> @@ -4082,6 +4100,31 @@ virDomainActualNetDefParseXML(xmlNodePtr node,
>              (!(actual->data.direct.virtPortProfile =
>                 virNetDevVPortProfileParse(virtPortNode))))
>              goto error;
> +    } else if (actual->type == VIR_DOMAIN_NET_TYPE_HOSTDEV) {
> +        xmlNodePtr virtPortNode = virXPathNode("./virtualport", ctxt);
> +        virDomainHostdevDefPtr hostdev = &actual->data.hostdev.def;
> +
> +        hostdev->parent.type = VIR_DOMAIN_DEVICE_NET;
> +        hostdev->parent.data.net = parent;
> +        hostdev->info = &parent->info;

Might be some tweaks here if you take my comments on 5/6 to have hostdev
track just a pointer, rather than a full parent struct.

> +        /* The helper function expects type to already be found and
> +         * passed in as a string, since it is in a different place in
> +         * NetDef vs HostdevDef.
> +         */
> +        addrtype = virXPathString("string(./source/address/@type)", ctxt);
> +        if ((!addrtype) && virXPathNode("./source/vendor", ctxt))
> +           addrtype = strdup("usb"); /* source/vendor implies usb device */

Indentation, and check for OOM.

> @@ -4397,6 +4446,27 @@ virDomainNetDefParseXML(virCapsPtr caps,
>  
>          break;
>  
> +    case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> +        hostdev = &def->data.hostdev.def;
> +        hostdev->parent.type = VIR_DOMAIN_DEVICE_NET;
> +        hostdev->parent.data.net = def;
> +        hostdev->info = &def->info;
> +        /* The helper function expects type to already be found and
> +         * passed in as a string, since it is in a different place in
> +         * NetDef vs HostdevDef.
> +         */
> +        addrtype = virXPathString("string(./source/address/@type)", ctxt);
> +        if ((!addrtype) && virXPathNode("./source/vendor", ctxt))
> +           addrtype = strdup("usb"); /* source/vendor implies usb device */

Again.

> +    case VIR_DOMAIN_NET_TYPE_HOSTDEV:
> +        virBufferAdjustIndent(buf, 8);
> +        if (virDomainHostdevSourceFormat(buf, &def->data.hostdev.def,
> +                                         flags, true) < 0) {
> +            return -1;
> +        }
> +        if (virNetDevVPortProfileFormat(def->data.hostdev.virtPortProfile,
> +                                        buf) < 0) {
> +            return -1;
> +        }
> +        virBufferAdjustIndent(buf, -8);
> +        break;
> +
> +
> +        if (virNetDevVPortProfileFormat(def->data.hostdev.virtPortProfile, buf) < 0)
> +            goto error;
> +        virBufferAdjustIndent(buf, -8);

Oops - duplicated code (two profile prints, and two unindents but only
one indent).

> +virDomainHostdevDefPtr
> +virDomainNetGetActualHostdev(virDomainNetDefPtr iface)
> +{
> +    if (iface->type == VIR_DOMAIN_NET_TYPE_HOSTDEV)
> +        return &iface->data.hostdev.def;
> +    else if ((iface->type == VIR_DOMAIN_NET_TYPE_NETWORK) &&
> +             (iface->data.network.actual->type
> +              == VIR_DOMAIN_NET_TYPE_HOSTDEV)) {

Style: you have 'if ... else if { ... }' with mismatched bracing.

Hmm, since the first if just calls return, you can s/else //, and you've
fixed the style problem without having to touch {}.

Looking forward to a continuation of this series.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]