[Kimchi-devel] [PATCH] Create xmlutils/qemucmdline.py to generate <qemu:commandline> XML

[Christy Perez]

Looks fine to me. Just one question below...

On 10/28/2014 12:27 PM, Aline Manera wrote:
> To use get_qemucmdline_xml() function you just need to pass a dict of
> {arg: value} as parameter.
> 
> Use the created function on vmtemplate.py instead of the plan text to
> get the <qemu:commandline> XML for the remote CDROM when libvirt does
> not support ISO streaming (but it is supported by QEMU).
> 
> Signed-off-by: Aline Manera <alinefm at linux.vnet.ibm.com>
> ---
>  src/kimchi/vmtemplate.py           | 28 ++++++++++--------------
>  src/kimchi/xmlutils/qemucmdline.py | 45 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 57 insertions(+), 16 deletions(-)
>  create mode 100644 src/kimchi/xmlutils/qemucmdline.py
> 
> diff --git a/src/kimchi/vmtemplate.py b/src/kimchi/vmtemplate.py
> index 8013a6f..41c8491 100644
> --- a/src/kimchi/vmtemplate.py
> +++ b/src/kimchi/vmtemplate.py
> @@ -34,8 +34,7 @@ from kimchi.imageinfo import probe_image, probe_img_info
>  from kimchi.isoinfo import IsoImage
>  from kimchi.utils import check_url_path, pool_name_from_uri
>  from kimchi.xmlutils.disk import get_disk_xml
> -
> -QEMU_NAMESPACE = "xmlns:qemu='http://libvirt.org/schemas/domain/qemu/1.0'"
> +from kimchi.xmlutils.qemucmdline import get_qemucmdline_xml
> 
> 
>  class VMTemplate(object):
> @@ -131,17 +130,6 @@ class VMTemplate(object):
>          params['index'] = self.info['cdrom_index']
>          params['path'] = self.info['cdrom']
> 
> -        qemu_stream_cmdline = """
> -            <qemu:commandline>
> -              <qemu:arg value='-drive'/>
> -              <qemu:arg value='file=%(path)s,if=none,id=drive-%(bus)s0-1-0,\
> -readonly=on,format=%(format)s'/>
> -              <qemu:arg value='-device'/>
> -              <qemu:arg value='%(bus)s-cd,bus=%(bus)s.1,unit=0,\
> -drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
> -            </qemu:commandline>
> -        """
> -
>          hostname = urlparse.urlparse(params['path']).hostname
>          if hostname is not None and not qemu_stream_dns:
>              ip = socket.gethostbyname(hostname)
> @@ -150,8 +138,17 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
>          if self.info.get('iso_stream', False):
>              protocol = urlparse.urlparse(params['path']).scheme
>              if protocol not in libvirt_stream_protocols:
> +                driveOpt = 'file=%(path)s,if=none,id=drive-%(bus)s0-1-0,'
> +                driveOpt += 'readonly=on,format=%(format)s'
> +
> +                deviceOpt = '%(bus)s-cd,bus=%(bus)s.1,unit=0,'
> +                deviceOpt += 'drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'
> +
> +                args = {}
> +                args['-drive'] = driveOpt % params
> +                args['-device'] = deviceOpt % params

Should this bit have gone into the other patchset?

>                  # return qemucmdline XML
> -                return qemu_stream_cmdline % params
> +                return get_qemucmdline_xml(args)
> 
>          dev, xml = get_disk_xml(params)
>          return xml
> @@ -358,13 +355,12 @@ drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
>          if not urlparse.urlparse(self.info.get('cdrom', "")).scheme in \
>                  libvirt_stream_protocols and \
>                  params.get('iso_stream', False):
> -            params['qemu-namespace'] = QEMU_NAMESPACE
>              params['qemu-stream-cmdline'] = cdrom_xml
>          else:
>              params['cdroms'] = cdrom_xml
> 
>          xml = """
> -        <domain type='%(domain)s' %(qemu-namespace)s>
> +        <domain type='%(domain)s'>
>            %(qemu-stream-cmdline)s
>            <name>%(name)s</name>
>            <uuid>%(uuid)s</uuid>
> diff --git a/src/kimchi/xmlutils/qemucmdline.py b/src/kimchi/xmlutils/qemucmdline.py
> new file mode 100644
> index 0000000..66238a7
> --- /dev/null
> +++ b/src/kimchi/xmlutils/qemucmdline.py
> @@ -0,0 +1,45 @@
> +#
> +# Project Kimchi
> +#
> +# Copyright IBM, Corp. 2014
> +#
> +# 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 ElementMaker
> +
> +QEMU_NAMESPACE = "http://libvirt.org/schemas/domain/qemu/1.0"
> +
> +
> +def get_qemucmdline_xml(args):
> +    """
> +    <qemu:commandline xmlns:qemu="http://libvirt.org/schemas/domain/qemu/1.0">
> +      <qemu:arg value='-drive'/>
> +      <qemu:arg value='file=%(path)s,if=none,id=drive-%(bus)s0-1-0,
> +                readonly=on,format=%(format)s'/>
> +      <qemu:arg value='-device'/>
> +      <qemu:arg value='%(bus)s-cd,bus=%(bus)s.1,unit=0,
> +                drive=drive-%(bus)s0-1-0,id=%(bus)s0-1-0'/>
> +    </qemu:commandline>
> +    """
> +    EM = ElementMaker(namespace=QEMU_NAMESPACE,
> +                      nsmap={'qemu': QEMU_NAMESPACE})
> +
> +    root = EM.commandline()
> +    for opt, value in args.iteritems():
> +        root.append(EM.arg(value=opt))
> +        root.append(EM.arg(value=value))
> +
> +    return ET.tostring(root, encoding='utf-8', pretty_print=True)
>