[Kimchi-devel] [PATCH] [Kimchi 2/3] Add support to create guests to netboot.

Aline Manera alinefm at linux.vnet.ibm.com
Mon Apr 18 19:34:26 UTC 2016



On 04/18/2016 03:30 PM, Paulo Ricardo Paz Vital wrote:
> On Apr 18 01:14PM, Aline Manera wrote:
>>
>> On 04/15/2016 03:54 PM, pvital at linux.vnet.ibm.com wrote:
>>> From: Paulo Vital <pvital at linux.vnet.ibm.com>
>>>
>>> Add support to create guests without CD-ROM device information and setting the
>>> second boot order option as 'netboot'.
>>>
>>> This is part of solution to Issue #372.
>>>
>>> Signed-off-by: Paulo Vital <pvital at linux.vnet.ibm.com>
>>> ---
>>>   vmtemplate.py         | 25 ++++++++++++++-----------
>>>   xmlutils/bootorder.py | 45 +++++++++++++++++++++++++++++++++++++++++++++
>>>   2 files changed, 59 insertions(+), 11 deletions(-)
>>>   create mode 100644 xmlutils/bootorder.py
>>>
>>> diff --git a/vmtemplate.py b/vmtemplate.py
>>> index a223beb..17f34e5 100644
>>> --- a/vmtemplate.py
>>> +++ b/vmtemplate.py
>>> @@ -33,6 +33,7 @@ from wok.plugins.kimchi import imageinfo
>>>   from wok.plugins.kimchi import osinfo
>>>   from wok.plugins.kimchi.isoinfo import IsoImage
>>>   from wok.plugins.kimchi.utils import check_url_path, pool_name_from_uri
>>> +from wok.plugins.kimchi.xmlutils.bootorder import get_bootorder_xml
>>>   from wok.plugins.kimchi.xmlutils.cpu import get_cpu_xml
>>>   from wok.plugins.kimchi.xmlutils.disk import get_disk_xml
>>>   from wok.plugins.kimchi.xmlutils.graphics import get_graphics_xml
>>> @@ -146,9 +147,6 @@ class VMTemplate(object):
>>>                       d_info = imageinfo.probe_img_info(d['base'])
>>>                       d['size'] = d_info['virtual-size']
>>> -        if len(base_imgs) == 0:
>>> -            raise MissingParameter("KCHTMPL0016E")
>>> -
>> Why did you remove the above block?
>>
> The method of this block is responsible to get the OS info from the given ISO
> or image file while creating the VMTemplate instance. Since the idea is create
> a netboot template, I removed the check and raise to always return distro and
> version as unknow if no file is provided by user.

OK. But you need to make sure the template is for netboot, otherwise we 
should continue to raise the error.
Silence the error may lead on invalid template creation.

> This is also a backend solution to the Issue #931 (while UI is not reflecting
> the new API changes).

The root cause of #931 is not related to this patch or this solution. I 
have discussed with Ramon the Kimchi is not merging the disks 
information properly.



>>>           return distro, version
>>>
>>>       def _gen_name(self, distro, version):
>>> @@ -170,7 +168,7 @@ class VMTemplate(object):
>>>
>>>       def _get_cdrom_xml(self, libvirt_stream_protocols):
>>>           if 'cdrom' not in self.info:
>>> -            return ''
>>> +            return None
>>>
>>>           params = {}
>>>           params['type'] = 'cdrom'
>>> @@ -350,12 +348,18 @@ class VMTemplate(object):
>>>           libvirt_stream_protocols = kwargs.get('libvirt_stream_protocols', [])
>>>           cdrom_xml = self._get_cdrom_xml(libvirt_stream_protocols)
>>>
>>> -        if not urlparse.urlparse(self.info.get('cdrom', "")).scheme in \
>>> -                libvirt_stream_protocols and \
>>> -                params.get('iso_stream', False):
>>> -            params['qemu-stream-cmdline'] = cdrom_xml
>>> +        # Add information of CD-ROM device only if template have info about it.
>>> +        # Also, set up correct boot order.
>>> +        if cdrom_xml is not None:
>>> +            if not urlparse.urlparse(self.info.get('cdrom', "")).scheme in \
>>> +                    libvirt_stream_protocols and \
>>> +                    params.get('iso_stream', False):
>>> +                params['qemu-stream-cmdline'] = cdrom_xml
>>> +            else:
>>> +                params['cdroms'] = cdrom_xml
>>> +            params['boot_order'] = get_bootorder_xml()
>>>           else:
>>> -            params['cdroms'] = cdrom_xml
>>> +            params['boot_order'] = get_bootorder_xml(network=True)
>>>           # Setting maximum number of slots to avoid errors when hotplug memory
>>>           # Number of slots are the numbers of chunks of 1GB that fit inside
>>> @@ -409,8 +413,7 @@ class VMTemplate(object):
>>>             %(cpu_info_xml)s
>>>             <os>
>>>               <type arch='%(arch)s'>hvm</type>
>>> -            <boot dev='hd'/>
>>> -            <boot dev='cdrom'/>
>>> +            %(boot_order)s
>>>             </os>
>>>             <features>
>>>               <acpi/>
>>> diff --git a/xmlutils/bootorder.py b/xmlutils/bootorder.py
>>> new file mode 100644
>>> index 0000000..3f364f0
>>> --- /dev/null
>>> +++ b/xmlutils/bootorder.py
>>> @@ -0,0 +1,45 @@
>>> +#
>>> +# Project Kimchi
>>> +#
>>> +# Copyright IBM Corp, 2016
>>> +#
>>> +# This library is free software; you can redistribute it and/or
>>> +# modify it under the terms of the GNU Lesser General Public
>>> +# License as published by the Free Software Foundation; either
>>> +# version 2.1 of the License, or (at your option) any later version.
>>> +#
>>> +# This library is distributed in the hope that it will be useful,
>>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> +# Lesser General Public License for more details.
>>> +#
>>> +# You should have received a copy of the GNU Lesser General Public
>>> +# License along with this library; if not, write to the Free Software
>>> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301 USA
>>> +
>>> +import lxml.etree as ET
>>> +from lxml.builder import E
>>> +
>>> +
>>> +def get_bootorder_xml(network=False):
>>> +    """
>>> +    <boot dev='hd'/>
>>> +    <boot dev='cdrom'/>
>>> +
>>> +    - For netboot configuration:
>>> +
>>> +    <boot dev='hd'/>
>>> +    <boot dev='cdrom'/>
>>> +    <boot dev='network'/>
>>> +    """
>>> +    boot_order = ['hd', 'cdrom']
>>> +
>>> +    if network:
>>> +        boot_order.append('network')
>>> +
>> Why not always add 'network' option as default?
> That's OK to do, since this will not impact with guest has a disk
> or cdrom set up.
>
> Will remove the new file and change vmtemplate.py

in future, it would be good to have a boot order setup to allow user 
changes that when needed. There is already an issue open to cover it.
So if you want, I am OK to keep this file, but you can receive a list 
with the boot order parameters and return the XML. That way, we can 
reuse it in future.

get_bootorder_xml([hd, cdrom, network])

What do you think?

>>> +    boot_xml = ''
>>> +    for device in boot_order:
>>> +        boot = E.boot(dev=device)
>>> +        boot_xml += ET.tostring(boot, encoding='utf-8', pretty_print=True)
>>> +
>>> +    return boot_xml




More information about the Kimchi-devel mailing list