[virt-tools-list] [virt-manager PATCH 2/2] cli: fix cpu secure option to actually work

[Pavel Hrdina]

On Wed, May 22, 2019 at 10:24:47AM -0400, Cole Robinson wrote:
> On 5/22/19 8:15 AM, Pavel Hrdina wrote:
> > The 'secure' option is processed after the model is already set.
> > CPU security options are resolved while setting CPU model so we need
> > to know the 'secure' option value before we set the CPU model.
> > 
> > Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
> > ---
> > 
> 
> The cli options are applied to the XMLBuilder object in the order they
> are listed in the parser class. So rather than peak into optdict, this
> works in my testing:
> 
> diff --git a/virtinst/cli.py b/virtinst/cli.py
> index e137fb14..7b27e390 100644
> --- a/virtinst/cli.py
> +++ b/virtinst/cli.py
> @@ -1923,9 +1923,6 @@ class ParserCPU(VirtCLIParser):
>          return cb(inst, *args, **kwargs)
> 
>      def set_model_cb(self, inst, val, virtarg):
> -        if "secure" in self.optdict:
> -            inst.secure = _on_off_convert("secure", self.optdict["secure"])
> -
>          if val == "host":
>              val = inst.SPECIAL_MODE_HOST_MODEL
>          if val == "none":
> @@ -1954,11 +1951,11 @@ class ParserCPU(VirtCLIParser):
>      @classmethod
>      def _init_class(cls, **kwargs):
>          VirtCLIParser._init_class(**kwargs)
> +        cls.add_arg("secure", "secure", is_onoff=True)
>          cls.add_arg("model", "model", cb=cls.set_model_cb)
>          cls.add_arg("mode", "mode")
>          cls.add_arg("match", "match")
>          cls.add_arg("vendor", "vendor")
> -        cls.add_arg("secure", "secure", is_onoff=True)
>          cls.add_arg("cache.mode", "cache.mode")
>          cls.add_arg("cache.level", "cache.level")

Perfect, I did not like my solution and hoped that you might have some
advice since you've been cleaning the CLI parser recently.  I'll go with
that.

> > The ideal fix would be to refactor CLI parsing to introduce some
> > post-parse callback where we could properly set everything based
> > on the user input.
> > 
> 
> Yeah in general option ordering can't fix all issues, we can use
> set_defaults to sort some complex deps out too. The big problem is using
> virt-xml, lots of our option interactions are really only correctly
> handled at XML build time and not XML edit time, but I haven't really
> figured out a general way to resolve that

We might need to look into that someday.

I'll change the patch and push it.

Thanks,
Pavel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/virt-tools-list/attachments/20190522/c77ca910/attachment.sig>