From patchwork Thu Feb 19 02:49:34 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Han, Weidong" X-Patchwork-Id: 7906 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n1J2nvIw021445 for ; Thu, 19 Feb 2009 02:49:57 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752001AbZBSCtm (ORCPT ); Wed, 18 Feb 2009 21:49:42 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1753144AbZBSCtm (ORCPT ); Wed, 18 Feb 2009 21:49:42 -0500 Received: from mga02.intel.com ([134.134.136.20]:45034 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752001AbZBSCtl (ORCPT ); Wed, 18 Feb 2009 21:49:41 -0500 Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 18 Feb 2009 18:44:48 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.38,231,1233561600"; d="scan'208,223";a="387991486" Received: from azsmsx602.amr.corp.intel.com ([10.2.121.201]) by orsmga002.jf.intel.com with ESMTP; 18 Feb 2009 18:58:27 -0800 Received: from pdsmsx601.ccr.corp.intel.com (172.16.12.94) by azsmsx602.amr.corp.intel.com (10.2.121.201) with Microsoft SMTP Server (TLS) id 8.1.311.2; Wed, 18 Feb 2009 19:49:39 -0700 Received: from pdsmsx503.ccr.corp.intel.com ([172.16.12.95]) by pdsmsx601.ccr.corp.intel.com ([172.16.12.94]) with mapi; Thu, 19 Feb 2009 10:49:38 +0800 From: "Han, Weidong" To: "'Marcelo Tosatti'" CC: "'Avi Kivity'" , "'kvm@vger.kernel.org'" , "'Mark McLoughlin'" Date: Thu, 19 Feb 2009 10:49:34 +0800 Subject: RE: [PATCH 7/8] kvm: qemu: deassign device from guest Thread-Topic: [PATCH 7/8] kvm: qemu: deassign device from guest Thread-Index: AcmR7P4Q/kZ6zdTLQBKjCKCexoDcTwAQC3ig Message-ID: <715D42877B251141A38726ABF5CABF2C0195A1FC47@pdsmsx503.ccr.corp.intel.com> References: <715D42877B251141A38726ABF5CABF2C0195A1F9B7@pdsmsx503.ccr.corp.intel.com> <20090218171819.GB25719@amt.cnet> In-Reply-To: <20090218171819.GB25719@amt.cnet> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: yes X-MS-TNEF-Correlator: acceptlanguage: en-US MIME-Version: 1.0 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Marcelo Tosatti wrote: > Weidong, > > Does this set fix > > http://sourceforge.net/tracker2/?func=detail&aid=2432316&group_id=180599&atid=893831 > I found above bug was already gone even without my patch. I guess it's fixed by Mark: commit: 02874f4272b6787ff94ee7256ef083257b9d1eb1 Author: Mark McLoughlin Date: Fri Nov 28 17:10:47 2008 +0000 kvm: qemu: device-assignment: free device if hotplug fails Signed-off-by: Mark McLoughlin Signed-off-by: Avi Kivity Actually, my patch just moves free_assigned_device into init_assigned_device, no functional change. But I updated the patch to also call free_assigned_device when pci_register_device fails in init_assigned_device, because adev is allocated by qemu_mallocz in add_assigned_device. From ce48b0d6c636d8f49bc5977d1d144fa047273846 Mon Sep 17 00:00:00 2001 From: Weidong Han Date: Thu, 19 Feb 2009 10:49:30 +0800 Subject: [PATCH] kvm: qemu: free device on error in init_assigned_device make init_assigned_device call free_assigned_device on error, and then make free_assigned_device is static because it's only invoked in device-assigned.c. Acked-by: Mark McLoughlin Signed-off-by: Weidong Han --- qemu/hw/device-assignment.c | 14 +++++++++----- qemu/hw/device-assignment.h | 1 - qemu/hw/pci-hotplug.c | 1 - 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/qemu/hw/device-assignment.c b/qemu/hw/device-assignment.c index e6d2352..0b96ee4 100644 --- a/qemu/hw/device-assignment.c +++ b/qemu/hw/device-assignment.c @@ -443,7 +443,7 @@ again: static LIST_HEAD(, AssignedDevInfo) adev_head; -void free_assigned_device(AssignedDevInfo *adev) +static void free_assigned_device(AssignedDevInfo *adev) { AssignedDevice *dev = adev->assigned_dev; @@ -550,7 +550,7 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus) if (NULL == dev) { fprintf(stderr, "%s: Error: Couldn't register real device %s\n", __func__, adev->name); - return NULL; + goto out; } adev->assigned_dev = dev; @@ -558,14 +558,14 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus) if (get_real_device(dev, adev->bus, adev->dev, adev->func)) { fprintf(stderr, "%s: Error: Couldn't get real device (%s)!\n", __func__, adev->name); - return NULL; + goto out; } /* handle real device's MMIO/PIO BARs */ if (assigned_dev_register_regions(dev->real_device.regions, dev->real_device.region_number, dev)) - return NULL; + goto out; /* handle interrupt routing */ e_device = (dev->dev.devfn >> 3) & 0x1f; @@ -595,10 +595,14 @@ struct PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus) if (r < 0) { fprintf(stderr, "Failed to assign device \"%s\" : %s\n", adev->name, strerror(-r)); - return NULL; + goto out; } return &dev->dev; + +out: + free_assigned_device(adev); + return NULL; } /* diff --git a/qemu/hw/device-assignment.h b/qemu/hw/device-assignment.h index f216bb0..6a9b9fa 100644 --- a/qemu/hw/device-assignment.h +++ b/qemu/hw/device-assignment.h @@ -94,7 +94,6 @@ struct AssignedDevInfo { int disable_iommu; }; -void free_assigned_device(AssignedDevInfo *adev); PCIDevice *init_assigned_device(AssignedDevInfo *adev, PCIBus *bus); AssignedDevInfo *add_assigned_device(const char *arg); void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices); diff --git a/qemu/hw/pci-hotplug.c b/qemu/hw/pci-hotplug.c index 8c76453..65fafd1 100644 --- a/qemu/hw/pci-hotplug.c +++ b/qemu/hw/pci-hotplug.c @@ -143,7 +143,6 @@ static PCIDevice *qemu_pci_hot_assign_device(PCIBus *pci_bus, const char *opts) ret = init_assigned_device(adev, pci_bus); if (ret == NULL) { term_printf("Failed to assign device\n"); - free_assigned_device(adev); return NULL; }