Hi Chen, thanks for the patch! Sorry for the review delay, comments inline. On 11/12/2012 03:12 AM, Chen Hanxiao wrote: > From: ChenHanxiao <chenhanxiao@xxxxxxxxxxxxxx> > > This patch will add virtio-scsi bus option on "Add New Virtual > Hardware" GUI page. It will support users to add a virtual disk > using SCSI bus with a controller model virtio-scsi. > In virt-manager, we will provide virtio-scsi disk XML for libvirt: > > <disk type='block' device='disk'> > <driver name='qemu'/> > <source dev='/dev/sdb'/> > <target dev='sda' bus='scsi' model='virtio-scsi'/> > </disk> > > We provided tag 'model' for libvirt to determine which SCSI > controller model to use. > > Signed-off-by: ChenHanxiao <chenhanxiao@xxxxxxxxxxxxxx> > --- > src/virtManager/addhardware.py | 7 +++++++ > src/virtManager/details.py | 6 +++++- > 2 files changed, 12 insertions(+), 1 deletions(-) > > diff --git a/src/virtManager/addhardware.py b/src/virtManager/addhardware.py > index 4d894eb..9a7602c 100644 > --- a/src/virtManager/addhardware.py > +++ b/src/virtManager/addhardware.py > @@ -539,6 +539,8 @@ class vmmAddHardware(vmmGObjectUI): > if self.vm.get_hv_type() == "kvm": > add_dev("sata", virtinst.VirtualDisk.DEVICE_DISK, "SATA disk") > add_dev("virtio", virtinst.VirtualDisk.DEVICE_DISK, "Virtio disk") > + add_dev("virtio-scsi", virtinst.VirtualDisk.DEVICE_DISK, > + "Virtio-scsi disk") That last string is a disk label, so please make it something like _('Virtio SCSI disk') > if self.conn.is_xen(): > add_dev("xen", virtinst.VirtualDisk.DEVICE_DISK, "Virtual disk") > > @@ -1222,6 +1224,10 @@ class vmmAddHardware(vmmGObjectUI): > bus, device = self.get_config_disk_target() > cache = self.get_config_disk_cache() > fmt = self.get_config_disk_format() > + model = None > + if bus == "virtio-scsi": > + bus = "scsi" > + model = "virtio-scsi" > > # Make sure default pool is running > if self.is_default_storage(): > @@ -1271,6 +1277,7 @@ class vmmAddHardware(vmmGObjectUI): > device=device, > bus=bus, > driverCache=cache, > + model=model, > format=fmt) > > if not fmt: > diff --git a/src/virtManager/details.py b/src/virtManager/details.py > index d20e748..0e86648 100644 > --- a/src/virtManager/details.py > +++ b/src/virtManager/details.py > @@ -2219,7 +2219,11 @@ class vmmDetails(vmmGObjectUI): > if bus == "spapr-vscsi": > bus = "scsi" > addr = "spapr-vio" > - add_define(self.vm.define_disk_bus, dev_id_info, bus, addr) > + if bus == "virtio-scsi": > + bus = "scsi" > + model = "virtio-scsi" > + add_define(self.vm.define_disk_bus, dev_id_info, bus, > + addr, model) > > return self._change_config_helper(df, da, hf, ha) > > Does this actually work? By my reading this breaks all disk adding because model is only defined for bus == "virtio-scsi", and define_disk_bus in domain.py wasn't changed to accept the model parameter. An easy way to test all this stuff is to do virt-manager --connect test:///default That uses the libvirt stub driver which will allow you to add all manner of devices to a fake guest and nothing will affect the local machine. I've added a few commits to help with that, so make sure you rebase your changes on latest virt-manager git. - Cole _______________________________________________ virt-tools-list mailing list virt-tools-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/virt-tools-list