[libvirt] [PATCH] conf: Don't parse edev of non-passthrough input device
In input devices, edev attribute is only for passthrough devices. Don't parse this for other input devices. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1591151 Signed-off-by: Han Han <hhan@redhat.com> --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7396616eda..ea2e796b78 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13050,7 +13050,8 @@ virDomainInputDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } - if ((evdev = virXPathString("string(./source/@evdev)", ctxt))) + if ((evdev = virXPathString("string(./source/@evdev)", ctxt)) && + (def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH)) def->source.evdev = virFileSanitizePath(evdev); if (def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH && !def->source.evdev) { virReportError(VIR_ERR_XML_ERROR, "%s", -- 2.17.1
On 07/11/2018 01:57 AM, Han Han wrote:
In input devices, edev attribute is only for passthrough devices. Don't parse this for other input devices.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1591151
Signed-off-by: Han Han <hhan@redhat.com> --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7396616eda..ea2e796b78 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13050,7 +13050,8 @@ virDomainInputDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; }
- if ((evdev = virXPathString("string(./source/@evdev)", ctxt))) + if ((evdev = virXPathString("string(./source/@evdev)", ctxt)) && + (def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH)) def->source.evdev = virFileSanitizePath(evdev); if (def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH && !def->source.evdev) { virReportError(VIR_ERR_XML_ERROR, "%s",
Does this really change anything on the command line? That is before this patch does the path show up on the command line (no, I believe) and after? (no, of course). Look at commit id 1a538a07c which implemented this for domain/conf/xml and commit id 971f5f229 which generates the command line. All your change does is not parse the path for anything other than PASSTHROUGH leaving no way for a consumer to know they generated incorrect XML. Since this is a config/xml error, then you'll need to modify: virDomainDeviceDefValidateInternal and the case VIR_DOMAIN_DEVICE_INPUT: to add a new call such as: return virDomainInputDefValidate(dev->data.input); where the virDomainInputDefValidate would have a similar switch as virDomainInputDefGetPath, but if it finds input->source.evdev "set" for MOUSE, TABLET, or KBD, then an error should be generated, e.g.: virReportError(VIR_ERR_XML_ERROR, "%s", _("setting source evdev path only supported for " "passthrough input devices")); return -1; If a domain is created/defined/started using the bad XML format, then an error would be generated. For a running domain with bad XML it won't disappear. John
On Thu, Jul 19, 2018 at 12:34 AM, John Ferlan <jferlan@redhat.com> wrote:
On 07/11/2018 01:57 AM, Han Han wrote:
In input devices, edev attribute is only for passthrough devices. Don't parse this for other input devices.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1591151
Signed-off-by: Han Han <hhan@redhat.com> --- src/conf/domain_conf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7396616eda..ea2e796b78 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13050,7 +13050,8 @@ virDomainInputDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; }
- if ((evdev = virXPathString("string(./source/@evdev)", ctxt))) + if ((evdev = virXPathString("string(./source/@evdev)", ctxt)) && + (def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH)) def->source.evdev = virFileSanitizePath(evdev); if (def->type == VIR_DOMAIN_INPUT_TYPE_PASSTHROUGH && !def->source.evdev) { virReportError(VIR_ERR_XML_ERROR, "%s",
Does this really change anything on the command line? That is before this patch does the path show up on the command line (no, I believe) and after? (no, of course).
Look at commit id 1a538a07c which implemented this for domain/conf/xml and commit id 971f5f229 which generates the command line.
All your change does is not parse the path for anything other than PASSTHROUGH leaving no way for a consumer to know they generated incorrect XML.
Since this is a config/xml error, then you'll need to modify:
virDomainDeviceDefValidateInternal
and the
case VIR_DOMAIN_DEVICE_INPUT:
to add a new call such as:
return virDomainInputDefValidate(dev->data.input);
where the virDomainInputDefValidate would have a similar switch as virDomainInputDefGetPath, but if it finds input->source.evdev "set" for MOUSE, TABLET, or KBD, then an error should be generated, e.g.:
virReportError(VIR_ERR_XML_ERROR, "%s", _("setting source evdev path only supported for " "passthrough input devices")); return -1;
If a domain is created/defined/started using the bad XML format, then an error would be generated. For a running domain with bad XML it won't disappear.
John
Thank you for review. The 2rd version: https://www.redhat.com/archives/libvir-list/2018-July/msg01753.html -- Best regards, ----------------------------------- Han Han Quality Engineer Redhat. Email: hhan@redhat.com Phone: +861065339333
participants (2)
-
Han Han -
John Ferlan