[libvirt] [PATCH 0/2] enhance capabilities mode hostdev process
hostdev has mode "capabilities" for LXC, from formatdomain.html: " Block / character devices from the host can be passed through to the guest using the hostdev element. This is only possible with container based virtualization. since after 1.0.1 for LXC " So forbid capabilities mode hostdev if domain is not LXC. The related bug is: https://bugzilla.redhat.com/show_bug.cgi?id=1111044 Jincheng Miao (2): conf: only accept capabilities mode hostdev in LXC. docs: fix some typos in formatdomain.html docs/formatdomain.html.in | 4 ++-- src/conf/domain_conf.c | 10 ++++++++-- 2 files changed, 10 insertions(+), 4 deletions(-) -- 1.8.3.1
hostdev has mode "capabilities" for LXC, from formatdomain.html: " Block / character devices from the host can be passed through to the guest using the hostdev element. This is only possible with container based virtualization. since after 1.0.1 for LXC " So forbid capabilities mode hostdev if domain is not LXC. The related bug is: https://bugzilla.redhat.com/show_bug.cgi?id=1111044 Signed-off-by: Jincheng Miao <jmiao@redhat.com> --- src/conf/domain_conf.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4114289..5ae6614 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9574,6 +9574,12 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; break; case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES: + if (vmdef->virtType != VIR_DOMAIN_VIRT_LXC) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported capabilities mode hostdev in %s"), + virDomainVirtTypeToString(vmdef->virtType)); + goto error; + } /* parse managed/mode/type, and the <source> element */ if (virDomainHostdevDefParseXMLCaps(node, ctxt, type, def) < 0) goto error; @@ -9596,7 +9602,7 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: if (def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && def->info->type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + virReportError(VIR_ERR_XML_DETAIL, "%s", _("PCI host devices must use 'pci' address type")); goto error; } @@ -9605,7 +9611,7 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, if (def->info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && virDomainHostdevAssignAddress(xmlopt, vmdef, def) < 0) { - virReportError(VIR_ERR_XML_ERROR, "%s", + virReportError(VIR_ERR_XML_DETAIL, "%s", _("SCSI host devices must have address specified")); goto error; } -- 1.8.3.1
On Thu, Jun 19, 2014 at 05:45:12PM +0800, Jincheng Miao wrote:
hostdev has mode "capabilities" for LXC, from formatdomain.html: " Block / character devices from the host can be passed through to the guest using the hostdev element. This is only possible with container based virtualization. since after 1.0.1 for LXC " So forbid capabilities mode hostdev if domain is not LXC.
The related bug is: https://bugzilla.redhat.com/show_bug.cgi?id=1111044
Signed-off-by: Jincheng Miao <jmiao@redhat.com> --- src/conf/domain_conf.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 4114289..5ae6614 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9574,6 +9574,12 @@ virDomainHostdevDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; break; case VIR_DOMAIN_HOSTDEV_MODE_CAPABILITIES: + if (vmdef->virtType != VIR_DOMAIN_VIRT_LXC) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported capabilities mode hostdev in %s"), + virDomainVirtTypeToString(vmdef->virtType)); + goto error; + }
No, the parser code is not supposed to do semantic validation like this. This kind of check should be done exclusively in the driver code which forms the command line args for the hypervisors which don't support it. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
----- Original Message -----
No, the parser code is not supposed to do semantic validation like this. This kind of check should be done exclusively in the driver code which forms the command line args for the hypervisors which don't support it.
Oh, I just understand why there is no hypervisor type check in domain_conf.c. I will move this check to qemuBuildCommandLine(). Thanks for your review. Best wishes, Jincheng Miao
Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
In section "Block / character devices" of "Host device assignment", the description of hostdev element has some error: For a block device, the type should be "storage", not "block"; For a character device, the type should be "misc", not "char". Signed-off-by: Jincheng Miao <jmiao@redhat.com> --- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 79b85d5..3075e16 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2932,8 +2932,8 @@ <dt><code>hostdev</code></dt> <dd>The <code>hostdev</code> element is the main container for describing host devices. For block/character device passthrough <code>mode</code> is - always "capabilities" and <code>type</code> is "block" for a block - device, "char" for a character device and "net" for a host network + always "capabilities" and <code>type</code> is "storage" for a block + device, "misc" for a character device and "net" for a host network interface. </dd> <dt><code>source</code></dt> -- 1.8.3.1
On 06/19/2014 03:45 AM, Jincheng Miao wrote:
In section "Block / character devices" of "Host device assignment", the description of hostdev element has some error:
For a block device, the type should be "storage", not "block"; For a character device, the type should be "misc", not "char".
Signed-off-by: Jincheng Miao <jmiao@redhat.com> --- docs/formatdomain.html.in | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
ACK and pushed.
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 79b85d5..3075e16 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2932,8 +2932,8 @@ <dt><code>hostdev</code></dt> <dd>The <code>hostdev</code> element is the main container for describing host devices. For block/character device passthrough <code>mode</code> is - always "capabilities" and <code>type</code> is "block" for a block - device, "char" for a character device and "net" for a host network + always "capabilities" and <code>type</code> is "storage" for a block + device, "misc" for a character device and "net" for a host network interface. </dd> <dt><code>source</code></dt>
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Jincheng Miao