[virt-tools-list] ostype in virtinst and libvirt

Cole Robinson crobinso at redhat.com
Tue Feb 16 14:47:52 UTC 2010


On 02/16/2010 09:08 AM, Cleber Rosa wrote:
> Folks,
> 
> I've been playing around with python-virtinst, with the primary goal of adding an initial round of LXC support to it.
>

Sweet, LXC support would be great.

> While reading the code, I've found that "os_type" is quite misleading. This is already recognized and one example is this comment in virtinst/Guest.py:
> 
>     # GAH! - installer.os_type = "hvm" or "xen" (aka xen paravirt)
>     #        guest.os_type     = "Solaris", "Windows", "Linux"
>     # FIXME: We should really rename this property to something else,
>     #        change it throughout the codebase for readability sake, but
>     #        maintain back compat.
> 
> libvirt also refers to "ostype" all over the place, one example is in conf/capabilities.c:
> 
>     * virCapabilitiesAddGuest:
>     * @caps: capabilities to extend
>     * @ostype: guest operating system type ('hvm' or 'xen')
> 
> I feel that, even prior to adding LXC support to python-virtinst, I would benefit from learning more about the code while fixing this. I would like to have feedback on a couple of questions:
> 
>    * Should libvirt be targeted also? Would this have any impact on ABI/API stability?

Like Dan said, this can't be changed for compat reasons

>    * Has a consensus been formed about alternative names for "ostype", "installer.os_type", "guest.os_type" and the like?
> 

I think os_type/ostype should continue to refer to the <os><type> field
in the domain XML, and we should change the guest.os_type to
'distro_type' (and os_variant to distro_variant). Even though this will
eventually be obsoleted by libosinfo integration, it will improve code
readability.

This should be as simple as s/_os_type/_distro_type/g in Guest.py, and
and adding

distro_type = property(get_distro_type, set_distro_type)
# Back compat
get_os_type = get_distro_type
set_os_type = set_distro_type

Probably also want to change virt-install to match.

Thanks,
Cole




More information about the virt-tools-list mailing list