conf: Don't parse edev of non-passthrough input device

[libvirt] [PATCH] conf: Don't parse edev of non-passthrough input device

Posted by Han Han 5 years, 9 months ago

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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] conf: Don't parse edev of non-passthrough input device

Posted by John Ferlan 5 years, 9 months ago


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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] conf: Don't parse edev of non-passthrough input device

Posted by Han Han 5 years, 9 months ago

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
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list