From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:45125) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RtM9M-0008BE-DV for qemu-devel@nongnu.org; Fri, 03 Feb 2012 11:38:14 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RtM9D-0007NJ-SY for qemu-devel@nongnu.org; Fri, 03 Feb 2012 11:38:12 -0500 Received: from mail-pz0-f45.google.com ([209.85.210.45]:59230) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RtM9D-0007NB-Ng for qemu-devel@nongnu.org; Fri, 03 Feb 2012 11:38:03 -0500 Received: by dadp14 with SMTP id p14so3673703dad.4 for ; Fri, 03 Feb 2012 08:38:02 -0800 (PST) Message-ID: <4F2C0D66.8080906@codemonkey.ws> Date: Fri, 03 Feb 2012 10:37:58 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1328201142-26145-1-git-send-email-pbonzini@redhat.com> <1328201142-26145-2-git-send-email-pbonzini@redhat.com> <4F2AC1D2.9090606@codemonkey.ws> <4F2AC7F0.7080204@redhat.com> <4F2ADDA6.4070603@codemonkey.ws> <0225E1B3-010A-4A76-852A-60B6DBD3C331@suse.de> In-Reply-To: <0225E1B3-010A-4A76-852A-60B6DBD3C331@suse.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 01/16] qdev: fix hot-unplug List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf Cc: Paolo Bonzini , qemu-devel@nongnu.org On 02/02/2012 01:07 PM, Alexander Graf wrote: > > On 02.02.2012, at 20:01, Anthony Liguori wrote: > >> On 02/02/2012 11:29 AM, Paolo Bonzini wrote: >>> On 02/02/2012 06:03 PM, Anthony Liguori wrote: >>>>> >>>> >>>> Is this still needed with qom-upstream.14? I fixed a bug on .14 that >>>> involved child properties that was making device-del sometimes fail. >>> >>> Not sure, I tried with .13 but, from the look of it, it should still be there. >>> Regarding the .13->.14 diff: >>> >>> - you need QTAILQ_FOREACH_SAFE in object_property_del_child. >> >> Ack. >> >>> >>> - you need to check for the existence of the non-aliased name when accessing the >>> alias table, because s390 does not have PCI. >> >> I don't think that's the right strategy as it means that s390 only works if we don't include the PCI objects in the build (regardless of whether it uses PCI). This would be defeated if/when we move to having all device objects in a single shared library used by all of the qemu executables. >> >> I'd prefer to just drop the aliases for s390. I don't see a lot of value in it and I don't think there are tons of s390 users that will be affected. > > The reason for the aliases is to make -drive and -net work. If you have alternatives to aliases there, I'm happy to go with them. Um, but I see (in s390-virtio.c): for(i = 0; i < nb_nics; i++) { NICInfo *nd = &nd_table[i]; DeviceState *dev; if (!nd->model) { nd->model = g_strdup("virtio"); } if (strcmp(nd->model, "virtio")) { fprintf(stderr, "S390 only supports VirtIO nics\n"); exit(1); } dev = qdev_create((BusState *)s390_bus, "virtio-net-s390"); qdev_set_nic_properties(dev, nd); qdev_init_nofail(dev); } /* Create VirtIO disk drives */ for(i = 0; i < MAX_BLK_DEVS; i++) { DriveInfo *dinfo; DeviceState *dev; dinfo = drive_get(IF_IDE, 0, i); if (!dinfo) { continue; } dev = qdev_create((BusState *)s390_bus, "virtio-blk-s390"); qdev_prop_set_drive_nofail(dev, "drive", dinfo->bdrv); qdev_init_nofail(dev); } So s390 totally ignores the -drive if parameter and will only accept virtio for -net. From what I can tell, it's not an issue. But if we need it, we can do: diff --git a/arch_init.h b/arch_init.h index 828256c..bfbd9e1 100644 --- a/arch_init.h +++ b/arch_init.h @@ -32,4 +32,9 @@ int tcg_available(void); int kvm_available(void); int xen_available(void); +static inline int target_get_arch(void) +{ + return arch_type; +} + #endif diff --git a/blockdev.c b/blockdev.c index 7e4c548..caa9205 100644 --- a/blockdev.c +++ b/blockdev.c @@ -565,7 +565,15 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) case IF_VIRTIO: /* add virtio block device */ opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0); - qemu_opt_set(opts, "driver", "virtio-blk"); + switch(target_get_arch()) { + case QEMU_ARCH_S390X: + qemu_opt_set(opts, "driver", "virtio-blk-s390"); + break; + default: + qemu_opt_set(opts, "driver", "virtio-blk-pci"); + break; + } + qemu_opt_set(opts, "drive", dinfo->id); if (devaddr) qemu_opt_set(opts, "addr", devaddr); diff --git a/blockdev.c b/blockdev.c index 7e4c548..caa9205 100644 --- a/blockdev.c +++ b/blockdev.c @@ -565,7 +565,15 @@ DriveInfo *drive_init(QemuOpts *opts, int default_to_scsi) case IF_VIRTIO: /* add virtio block device */ opts = qemu_opts_create(qemu_find_opts("device"), NULL, 0); - qemu_opt_set(opts, "driver", "virtio-blk"); + switch(target_get_arch()) { + case QEMU_ARCH_S390X: + qemu_opt_set(opts, "driver", "virtio-blk-s390"); + break; + default: + qemu_opt_set(opts, "driver", "virtio-blk-pci"); + break; + } + qemu_opt_set(opts, "drive", dinfo->id); if (devaddr) qemu_opt_set(opts, "addr", devaddr); Can you confirm what we actually need here? Regards, Anthony Liguori > > > Alex > >