On Mon, 2012-06-18 at 10:21 -0400, Chris Evich wrote: > Onkar, > > Thanks for the fix. Did you send the right patch? When applied, this > patch seems to do nothing. It adds back in the same lines it removes. > To be sure, I even took the before/after and diffed them to make sure I > wasn't missing something very subtle.
Ok, I've checked the patch and worked on it a little bit. I understood the fix Onkar made in the patch: 1) If storage_pool is used, set filename to None and complete virt_install_cmd in a certain way 2) if not, filename will be not none, and then the virt_install_cmd will be updated in such a way that it will set the 'bus' format in virt_install_cmd to use 'drive_format' as the bus option. The patch actually had a bug, it was referencing virt_utils.get_image_filename(image_params, root_dir) Which was moved to virt_storage. Anyway, I've noticed it could be safely replaced with the value of filename, so the final patch was: >From 2918a0f5252f3daca9792307fd56d2861e2342c4 Mon Sep 17 00:00:00 2001 From: Onkar Mahajan <onkar.n.maha...@linux.vnet.ibm.com> Date: Mon, 18 Jun 2012 16:31:54 +0000 Subject: [PATCH] virt.libvirt_vm: Fix VM class to use proper drive bus parameter This patch fixes this issue - https://github.com/autotest/autotest/issues/390 Signed-off-by: Onkar Mahajan <onkar.n.maha...@linux.vnet.ibm.com> --- client/virt/libvirt_vm.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/client/virt/libvirt_vm.py b/client/virt/libvirt_vm.py index 05762d8..d97ac0c 100644 --- a/client/virt/libvirt_vm.py +++ b/client/virt/libvirt_vm.py @@ -979,10 +979,8 @@ class VM(virt_vm.BaseVM): filename = virt_storage.get_image_filename(image_params, root_dir) if image_params.get("use_storage_pool") == "yes": filename = None - if image_params.get("boot_drive") == "no": - continue - virt_install_cmd += add_drive(help, - filename, + virt_install_cmd += add_drive(help, + filename, image_params.get("image_pool"), image_params.get("image_vol"), image_params.get("image_device"), @@ -993,6 +991,21 @@ class VM(virt_vm.BaseVM): image_params.get("drive_cache"), image_params.get("image_format")) + if image_params.get("boot_drive") == "no": + continue + if filename: + virt_install_cmd += add_drive(help, + filename, + None, + None, + None, + image_params.get("drive_format"), + None, + image_params.get("image_size"), + image_params.get("drive_sparse"), + image_params.get("drive_cache"), + image_params.get("image_format")) + if (params.get('unattended_delivery_method') != 'integrated' and not (self.driver_type == 'xen' and params.get('hvm_or_pv') == 'pv')): for cdrom in params.objects("cdroms"): -- 1.7.10.2 Bottom line, thanks Onkar for fixing this, but next time, please be careful and add a Signed-off-by: line in the patch, yours was missing. Anyway, I've applied the fixed patch to next, thanks! > On 06/18/2012 04:31 AM, Onkar N Mahajan wrote: > > This patch fixes this issue - > > https://github.com/autotest/autotest/issues/390 > > > > client/virt/libvirt_vm.py | 21 +++++++++++++++++---- > > 1 files changed, 17 insertions(+), 4 deletions(-) > > > > diff --git a/client/virt/libvirt_vm.py b/client/virt/libvirt_vm.py > > index c784f83..5bb315c 100644 > > --- a/client/virt/libvirt_vm.py > > +++ b/client/virt/libvirt_vm.py > > @@ -972,10 +972,8 @@ class VM(virt_vm.BaseVM): > > filename = virt_storage.get_image_filename(image_params, > > root_dir) > > if image_params.get("use_storage_pool") == "yes": > > filename = None > > - if image_params.get("boot_drive") == "no": > > - continue > > - virt_install_cmd += add_drive(help, > > - filename, > > + virt_install_cmd += add_drive(help, > > + filename, > > image_params.get("image_pool"), > > image_params.get("image_vol"), > > image_params.get("image_device"), > > @@ -986,6 +984,21 @@ class VM(virt_vm.BaseVM): > > image_params.get("drive_cache"), > > image_params.get("image_format")) > > > > + if image_params.get("boot_drive") == "no": > > + continue > > + if filename: > > + virt_install_cmd += add_drive(help, > > + > > virt_utils.get_image_filename(image_params, root_dir), > > + None, > > + None, > > + None, > > + image_params.get("drive_format"), > > + None, > > + image_params.get("image_size"), > > + image_params.get("drive_sparse"), > > + image_params.get("drive_cache"), > > + image_params.get("image_format")) > > + > > if (params.get('unattended_delivery_method') != 'integrated' and > > not (self.driver_type == 'xen' and params.get('hvm_or_pv') == > > 'pv')): > > for cdrom in params.objects("cdroms"): > > _______________________________________________ Autotest mailing list Autotest@test.kernel.org http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
