On 10/29/2014 10:55 AM, Aline Manera wrote:
On 10/28/2014 05:00 PM, Christy Perez wrote:
> 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(a)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?
Assuming the other patchset is "[PATCH 0/8 V2] Create a common function
to generate guest disk XML" which changes only the way
we generate the
disk XML.
Yep.
It keeps using the qemu commandline plan text XML when it is the
case.
And this patch set change the way we generate the qemu commandline
because that I did it in a separated patch.
But as I mentioned, this patch depends on "[PATCH 0/8 V2] Create a
common function to generate guest disk XML"
This just seemed out of place because it didn't seem to be related to
the qemu command-line portion of the XML. But I see now how they're
related.
>
>> # 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)
>>
> _______________________________________________
> Kimchi-devel mailing list
> Kimchi-devel(a)ovirt.org
>
http://lists.ovirt.org/mailman/listinfo/kimchi-devel
>