conf: fix virtType FromString check

Message ID b12fae95a92dd766c2571ceec7cde179e2ab03b0.1532618816.git.crobinso@redhat.com
State New
Headers show
Series
  • conf: fix virtType FromString check
Related show

Commit Message

Cole Robinson July 26, 2018, 3:30 p.m.
In the domain definition, virtType is virDomainVirtType which is
unsigned, so comparing against -1 doesn't work. Cast to int first

Fixes 8e2982b5767

Signed-off-by: Cole Robinson <crobinso@redhat.com>

---
This regressed in the past as well, when virtType was changed to
virDomainVirtType in 7383b8cc068, fixed by the follow up 5e06a4f063.
It's strange that virDomainVirtType is unsigned but VirtTypeFromString
can return -1... it should probably either work like virArch, or we
should switch virDomainVirtType to int in the DomainDef, but that
requires fixing a few other places too

 src/conf/domain_conf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.17.1

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

Comments

Daniel P. Berrangé July 26, 2018, 3:32 p.m. | #1
On Thu, Jul 26, 2018 at 11:30:07AM -0400, Cole Robinson wrote:
> In the domain definition, virtType is virDomainVirtType which is

> unsigned, so comparing against -1 doesn't work. Cast to int first

> 

> Fixes 8e2982b5767

> 

> Signed-off-by: Cole Robinson <crobinso@redhat.com>

> ---

> This regressed in the past as well, when virtType was changed to

> virDomainVirtType in 7383b8cc068, fixed by the follow up 5e06a4f063.

> It's strange that virDomainVirtType is unsigned but VirtTypeFromString

> can return -1... it should probably either work like virArch, or we

> should switch virDomainVirtType to int in the DomainDef, but that

> requires fixing a few other places too

> 

>  src/conf/domain_conf.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c

> index c27c874d9e..30806d1e59 100644

> --- a/src/conf/domain_conf.c

> +++ b/src/conf/domain_conf.c

> @@ -19140,7 +19140,7 @@ virDomainDefParseCaps(virDomainDefPtr def,

>                         "%s", _("missing domain type attribute"));

>          goto cleanup;

>      }

> -    if ((def->virtType = virDomainVirtTypeFromString(virttype)) < 0) {

> +    if ((int)(def->virtType = virDomainVirtTypeFromString(virttype)) < 0) {

>          virReportError(VIR_ERR_CONFIG_UNSUPPORTED,

>                         _("invalid domain type %s"), virttype);

>          goto cleanup;


I've already pushed the fix for this problem, changing "virtType"
back to an int.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

Patch

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index c27c874d9e..30806d1e59 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -19140,7 +19140,7 @@  virDomainDefParseCaps(virDomainDefPtr def,
                        "%s", _("missing domain type attribute"));
         goto cleanup;
     }
-    if ((def->virtType = virDomainVirtTypeFromString(virttype)) < 0) {
+    if ((int)(def->virtType = virDomainVirtTypeFromString(virttype)) < 0) {
         virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                        _("invalid domain type %s"), virttype);
         goto cleanup;