From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:58887) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QcYgL-00023T-4p for qemu-devel@nongnu.org; Fri, 01 Jul 2011 04:02:38 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QcYgF-0007FZ-GH for qemu-devel@nongnu.org; Fri, 01 Jul 2011 04:02:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:14716) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QcYgF-0007FS-0c for qemu-devel@nongnu.org; Fri, 01 Jul 2011 04:02:27 -0400 Message-ID: <4E0D7FB2.9010609@redhat.com> Date: Fri, 01 Jul 2011 10:05:06 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1309260558-3332-1-git-send-email-stefano.stabellini@eu.citrix.com> <4E0C681D.60001@redhat.com> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3] xen: implement unplug protocol in xen_platform List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefano Stabellini Cc: "xen-devel@lists.xensource.com" , "mst@redhat.com" , "armbru@redhat.com" , "qemu-devel@nongnu.org" , "agraf@suse.de" , Anthony Perard Am 30.06.2011 16:16, schrieb Stefano Stabellini: > On Thu, 30 Jun 2011, Kevin Wolf wrote: >>> +static int pci_piix3_xen_ide_unplug(DeviceState *dev) >>> +{ >>> + PCIDevice *pci_dev; >>> + PCIIDEState *pci_ide; >>> + DriveInfo *di; >>> + int i = 0; >>> + >>> + pci_dev = DO_UPCAST(PCIDevice, qdev, dev); >>> + pci_ide = DO_UPCAST(PCIIDEState, dev, pci_dev); >>> + >>> + for (; i < 3; i++) { >>> + di = drive_get_by_index(IF_IDE, i); >>> + if (di != NULL && di->bdrv != NULL && !di->bdrv->removable) { >>> + DeviceState *ds = bdrv_get_attached(di->bdrv); >>> + if (ds) { >>> + bdrv_detach(di->bdrv, ds); >>> + } >>> + bdrv_close(di->bdrv); >>> + pci_ide->bus[di->bus].ifs[di->unit].bs = NULL; >> >> Have you tested if this is enough if the guest tries to continue using >> the device? I don't know of any case where it's not sufficient, just >> trying to make sure that it's really true in practice. > > The purpose of this is to "hide" the disk from the guest. The unplug is > supposed to happen *before* the guest enumerates the IDE disks; it is > responsibility of the guest to make sure of it. > I tested it with Linux PV on HVM drivers, and Linux doesn't see the > emulated disk after the unplug, as it should be. Yeah. What I meant is that we should make sure that a misbehaving guest, which just keeps on playing with the IDE ports anyway, can't crash qemu. A quick review suggests that it is the case, but testing it anyway would be better. >>> + drive_put_ref(di); >>> + } >>> + } >>> + qdev_reset_all(&(pci_ide->dev.qdev)); >>> + return 0; >>> +} >>> + >>> +PCIDevice *pci_piix3_xen_ide_init(PCIBus *bus, DriveInfo **hd_table, int devfn) >>> +{ >>> + PCIDevice *dev; >>> + >>> + dev = pci_create_simple(bus, devfn, "piix3-ide-xen"); >>> + dev->qdev.info->unplug = pci_piix3_xen_ide_unplug; >> >> Can't this be moved into the PCIDeviceInfo now that we have a separate >> one for Xen? > > No because it would be overridden by the default pci unplug function, > that is not what we want in this case. Okay. I'm not familiar with that code, so I'll just trust you there. Kevin