On 11/30/2012 01:26 PM, Daniel P. Berrange wrote:
> From: "Daniel P. Berrange" <berrange@xxxxxxxxxx>
>
> The <hostdev> device type has long had a redundant "mode"
> attribute, which has always been "subsys". This finally
> introduces a new mode "capabilities", which will be used
> by the LXC driver for device assignment. Since container
> based virtualization uses a single kernel, the idea of
> assigning physical PCI devices doesn't make sense. It is
> still reasonable to assign USB devices, but for assinging
s/assinging/assigning/
> arbitrary nodes in /dev, the new 'capabilities' mode is
> to be used.
>
> The first capability support is 'storage', which is for
> assignment of block devices. Functionally this is really
> pretty similar to the <disk> support. The only difference
> is the device node name is identical in both host and
> container namespaces.
>
> <hostdev mode='capabilities' type='storage'>
> <source>
> <block>/dev/sdf1</block>
> </source>
> </hostdev>
>
> The second capability support is 'misc', which is for
> assignment of character devices. There is no existing
> parallel to this. Again the device node is the same
> inside & outside the container.
>
> <hostdev mode='capabilities' type='misc'>
> <source>
> <char>/dev/sdf1</char>
> </source>
> </hostdev>
>
> The reason for keeping the char & storage devices
> separate in the domain XML, is to mirror the split
> in the node device XML. NB the node device XML does
> not yet report character devices, but that's another
> new patch to come
>
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
> docs/schemas/domaincommon.rng | 128 +++++++++++++++++--------
Missing documentation in an appropriate docs/*.html.in. I haven't
checked if you add it later in the series, or completely forgot.
> src/conf/domain_audit.c | 80 +++++++++++-----
> src/conf/domain_conf.c | 175 ++++++++++++++++++++++++++++++++++-
> src/conf/domain_conf.h | 31 +++++--
> src/libvirt_private.syms | 1 +
> tests/lxcxml2xmldata/lxc-hostdev.xml | 34 +++++++
> tests/lxcxml2xmltest.c | 1 +
> 7 files changed, 375 insertions(+), 75 deletions(-)
> create mode 100644 tests/lxcxml2xmldata/lxc-hostdev.xml
But at least you added tests.
> +
> + <define name="hostdevcaps">
> + <attribute name="mode">
> + <value>capabilities</value>
> + </attribute>
> + <choice>
> + <group>
> + <ref name="hostdevcapsstorage"/>
> + </group>
> + </choice>
> + </define>
> + <define name="hostdevcapsstorage">
> + <attribute name="type">
> + <value>storage</value>
> + </attribute>
> + <element name="source">
> + <element name="block">
> + <ref name="absFilePath"/>
> + </element>
> + </element>
I see where you added <hostdev mode='capabilities' type='storage'> but
not where you added <hostdev mode='capabilities' type='misc'>. Did you
mean to have a <choice>storage|misc</choice> for the type attribute in
hostdevcapsstorage?
> +
> + case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES:
> + switch (hostdev->source.caps.type) {
> + case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE:
> + if (!(device = virAuditEncode("disk",
> + VIR_AUDIT_STR(hostdev->source.caps.u.storage.block)))) {
> + VIR_WARN("OOM while encoding audit message");
> + goto cleanup;
> + }
> +
> + VIR_AUDIT(VIR_AUDIT_RECORD_RESOURCE, success,
> + "virt=%s resrc=hostdev reason=%s %s uuid=%s %s",
> + virt, reason, vmname, uuidstr, device);
> + break;
> +
> + default:
> + VIR_WARN("Unexpected hostdev type while encoding audit message: %d",
> + hostdev->source.caps.type);
Are you also missing 'misc' handling in this section of C code? I see
places where it appears, but it seems incomplete (maybe I missed
something, though).
> - if (!(device = virAuditEncode("device", VIR_AUDIT_STR(address)))) {
> - VIR_WARN("OOM while encoding audit message");
> + default:
> + VIR_WARN("Unexpected hostdev mode while cndoing audit message: %d",
s/cndoing/encoding/
> +++ b/src/conf/domain_conf.c
> @@ -530,12 +530,16 @@ VIR_ENUM_IMPL(virDomainGraphicsSpiceClipboardCopypaste,
>
> VIR_ENUM_IMPL(virDomainHostdevMode, VIR_DOMAIN_HOSTDEV_MODE_LAST,
> "subsystem",
> - "capabilities")
> + "capabilities");
Why the added ';'...
>
> VIR_ENUM_IMPL(virDomainHostdevSubsys, VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST,
> "usb",
> "pci")
...especially when it's already optional?
> @@ -1440,6 +1444,17 @@ void virDomainHostdevDefClear(virDomainHostdevDefPtr def)
> */
> if (def->parent.type == VIR_DOMAIN_DEVICE_NONE)
> virDomainDeviceInfoFree(def->info);
> +
> + if (def->mode == VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES) {
> + switch (def->source.caps.type) {
> + case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_STORAGE:
> + VIR_FREE(def->source.caps.u.storage.block);
> + break;
> + case VIR_DOMAIN_HOSTDEV_CAPS_TYPE_MISC:
> + VIR_FREE(def->source.caps.u.misc.chardev);
> + break;
> + }
For example, here you do handle both types.
--
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list