[virt-tools-list] [virt-manager PATCH 1/2] guest: Default to libosinfo recommended resources

[Pavel Hrdina]

On Wed, Dec 05, 2018 at 09:15:18AM +0100, Fabiano Fidêncio wrote:
> Let's create a new method that defaults to libosinfo's recommended
> resources (when they're available) for memory and vcpus.
> 
> It'll help us to avoid erroring out whenever virt-install is called
> without specifying the memory amount, as the recommended amount of
> memory would come from libosinfo.
> 
> Signed-off-by: Fabiano Fidêncio <fidencio at redhat.com>
> ---
>  virtinst/guest.py | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/virtinst/guest.py b/virtinst/guest.py
> index eeb40cb6..ced8f259 100644
> --- a/virtinst/guest.py
> +++ b/virtinst/guest.py
> @@ -475,11 +475,11 @@ class Guest(XMLBuilder):
>      def set_defaults(self, _guest):
>          if not self.uuid:
>              self.uuid = util.generate_uuid(self.conn)
> -        if not self.vcpus:
> -            self.vcpus = 1
>  
>          self.set_capabilities_defaults()
>  
> +        self._set_default_resources()
> +

Nitpick: no need for the empty line.

>          self._set_default_machine()
>          self._set_default_uefi()
>  
> @@ -510,6 +510,18 @@ class Guest(XMLBuilder):
>      # Private xml routines #
>      ########################
>  
> +    def _set_default_resources(self):
> +        res = self.osinfo.get_recommended_resources(self)
> +        if not res:
> +            return
> +
> +        if not self.memory and res.get('ram') > 0:
> +            self.memory = res['ram'] // 1024

The .get() method for dictionary can have default value as second
parameter and if there is no second parameter the default is None.

So in this case it's not correct, I'm sure that if there is any 'ram'
value stored in osinfo-db it will never be '0' and the only case when
the 'res.get('ram') > 0' would fail is if there is no 'ram' value and in
that case python would complain about comparing None and Int.

It should be only:

    if not self.memory and res.get('ram')

> +
> +        if not self.vcpus:
> +            self.vcpus = res.get('n-cpus') if res.get('n-cpus') > 0 else 1
> +

Here we can use the default parameter for .get() so:

    self.vcpus = res.get('n-cpus', 1)

> +
>      def _set_default_machine(self):
>          if self.os.machine:
>              return
> -- 
> 2.19.1

This patch breaks tests, be sure to run all of these before posting
patches:

./setup.py build
./setup.py test
./setup.py pylint

and if you feel like it and have codespell installed

./setup.py codespell

For the test data, you can use './setup.py test --regenerate-output'
to update all XMLs, but review all the changes if it's correct.

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/20181207/7b93d485/attachment.sig>