[PATCH] bug fix: Allow user updates the cdrom from local to remote file
It depends on: - [Kimchi-devel] [PATCH v3 1/3] Fix Key Error when editing CD ROM path - [Kimchi-devel] [PATCH v3 2/3] Fix verification of remote ISO - [Kimchi-devel] [PATCH v3 3/3] Add unit tests for remote-backed CD ROM updates. And I still need to update the test cases accordingly. Aline Manera (1): bug fix: Allow user updates the cdrom from local to remote file src/kimchi/model/vmstorages.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) -- 1.9.3
dom.updateDeviceFlags() can not work well while updating cdrom configuration from local to remote file. Even passing a right xml to this API, it updates the vm xml like below: <disk type='network' device='cdrom'> <driver name='qemu' type='raw'/> <source protocol='nbd' name='/13.04/ubuntu-13.04-desktop-amd64.iso'/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> Instead of: <disk type='network' device='cdrom'> <driver name='qemu' type='raw'/> <source protocol='http' name='/13.04/ubuntu-13.04-desktop-amd64.iso'> <host name='ubuntu-releases.cs.umn.edu' port='80'/> </source> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk> In this case it is safe to manually update the vm xml to properly change the cdrom configuration instead of using the libvirt API. Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index b5311db..d2da4aa 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -250,6 +250,21 @@ class VMStorageModel(object): xml = _get_storage_xml(dev_info, ignore_source) try: + # FIXME: dom.updateDeviceFlags() can not work well while updating + # cdrom configuration from local to remote file + # In this case it is safe to manually update the vm xml to properly + # change the cdrom configuration + if dev_info['type'] == 'cdrom' and params['src_type'] == 'network': + # check if the current cdrom is a local file + conn = self.conn.get() + dom = conn.lookupByName(vm_name) + root = ET.fromstring(dom.XMLDesc(0)) + devices = root.find(".devices") + disk = devices.find(".disk/target[@dev='%s']/.." % dev_name) + if disk.attrib["type"] != 'network': + devices.replace(disk, ET.fromstring(xml)) + conn.defineXML(ET.tostring(root)) + dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0009E", {'error': e.message}) -- 1.9.3
I can reproduce this (insane)bug in Fedora20:(, will it be better to integrate it with vm model update logic to wrap this kind of fallback? That way code maintain would be easier. On 2014年08月12日 10:31, Aline Manera wrote:
dom.updateDeviceFlags() can not work well while updating cdrom configuration from local to remote file. Even passing a right xml to this API, it updates the vm xml like below:
<disk type='network' device='cdrom'> <driver name='qemu' type='raw'/> <source protocol='nbd' name='/13.04/ubuntu-13.04-desktop-amd64.iso'/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk>
Instead of:
<disk type='network' device='cdrom'> <driver name='qemu' type='raw'/> <source protocol='http' name='/13.04/ubuntu-13.04-desktop-amd64.iso'> <host name='ubuntu-releases.cs.umn.edu' port='80'/> </source> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk>
In this case it is safe to manually update the vm xml to properly change the cdrom configuration instead of using the libvirt API.
Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index b5311db..d2da4aa 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -250,6 +250,21 @@ class VMStorageModel(object): xml = _get_storage_xml(dev_info, ignore_source)
try: + # FIXME: dom.updateDeviceFlags() can not work well while updating + # cdrom configuration from local to remote file + # In this case it is safe to manually update the vm xml to properly + # change the cdrom configuration + if dev_info['type'] == 'cdrom' and params['src_type'] == 'network': + # check if the current cdrom is a local file + conn = self.conn.get() + dom = conn.lookupByName(vm_name) + root = ET.fromstring(dom.XMLDesc(0)) + devices = root.find(".devices") + disk = devices.find(".disk/target[@dev='%s']/.." % dev_name) + if disk.attrib["type"] != 'network': + devices.replace(disk, ET.fromstring(xml)) + conn.defineXML(ET.tostring(root)) + dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0009E", {'error': e.message})
On 08/12/2014 07:08 AM, Royce Lv wrote:
I can reproduce this (insane)bug in Fedora20:(, will it be better to integrate it with vm model update logic to wrap this kind of fallback? That way code maintain would be easier.
I don't think so. The vms model is related to /vms API and the problem is specific of /vms/<vm-name>/storage/<cdrom> I think it is better to keep it here (in vmstorages.py) and remove it when we have libvirt fixed.
On 2014年08月12日 10:31, Aline Manera wrote:
dom.updateDeviceFlags() can not work well while updating cdrom configuration from local to remote file. Even passing a right xml to this API, it updates the vm xml like below:
<disk type='network' device='cdrom'> <driver name='qemu' type='raw'/> <source protocol='nbd' name='/13.04/ubuntu-13.04-desktop-amd64.iso'/> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk>
Instead of:
<disk type='network' device='cdrom'> <driver name='qemu' type='raw'/> <source protocol='http' name='/13.04/ubuntu-13.04-desktop-amd64.iso'> <host name='ubuntu-releases.cs.umn.edu' port='80'/> </source> <target dev='hdc' bus='ide'/> <readonly/> <address type='drive' controller='0' bus='1' target='0' unit='0'/> </disk>
In this case it is safe to manually update the vm xml to properly change the cdrom configuration instead of using the libvirt API.
Signed-off-by: Aline Manera <alinefm@linux.vnet.ibm.com> --- src/kimchi/model/vmstorages.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/src/kimchi/model/vmstorages.py b/src/kimchi/model/vmstorages.py index b5311db..d2da4aa 100644 --- a/src/kimchi/model/vmstorages.py +++ b/src/kimchi/model/vmstorages.py @@ -250,6 +250,21 @@ class VMStorageModel(object): xml = _get_storage_xml(dev_info, ignore_source)
try: + # FIXME: dom.updateDeviceFlags() can not work well while updating + # cdrom configuration from local to remote file + # In this case it is safe to manually update the vm xml to properly + # change the cdrom configuration + if dev_info['type'] == 'cdrom' and params['src_type'] == 'network': + # check if the current cdrom is a local file + conn = self.conn.get() + dom = conn.lookupByName(vm_name) + root = ET.fromstring(dom.XMLDesc(0)) + devices = root.find(".devices") + disk = devices.find(".disk/target[@dev='%s']/.." % dev_name) + if disk.attrib["type"] != 'network': + devices.replace(disk, ET.fromstring(xml)) + conn.defineXML(ET.tostring(root)) + dom.updateDeviceFlags(xml, get_vm_config_flag(dom, 'all')) except Exception as e: raise OperationFailed("KCHVMSTOR0009E", {'error': e.message})
participants (2)
-
Aline Manera -
Royce Lv