All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Han, Weidong" <weidong.han@intel.com>
To: 'Marcelo Tosatti' <mtosatti@redhat.com>
Cc: 'Avi Kivity' <avi@redhat.com>,
	"'kvm@vger.kernel.org'" <kvm@vger.kernel.org>,
	'Mark McLoughlin' <markmc@redhat.com>
Subject: RE: [PATCH 7/8] kvm: qemu: deassign device from guest
Date: Thu, 19 Feb 2009 10:49:34 +0800	[thread overview]
Message-ID: <715D42877B251141A38726ABF5CABF2C0195A1FC47@pdsmsx503.ccr.corp.intel.com> (raw)
In-Reply-To: <20090218171819.GB25719@amt.cnet>

[-- Attachment #1: Type: text/plain, Size: 4812 bytes --]

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 <markmc@redhat.com>
Date:   Fri Nov 28 17:10:47 2008 +0000

    kvm: qemu: device-assignment: free device if hotplug fails
    
    Signed-off-by: Mark McLoughlin <markmc@redhat.com>
    Signed-off-by: Avi Kivity <avi@redhat.com>


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 <weidong.han@intel.com>
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 <markmc@redhat.com>
Signed-off-by: Weidong Han <weidong.han@intel.com>
---
 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;
     }
 
-- 
1.6.0.4



> 
> On Wed, Feb 18, 2009 at 03:13:05PM +0800, Han, Weidong wrote:
>> free_assigned_device just frees device from qemu, it should also
>> deassign the device from guest when guest exits or hot remove
>> assigned device. 
>> 
>> Acked-by: Mark McLoughlin <markmc@redhat.com>
>> Signed-off-by: Weidong Han <weidong.han@intel.com>
>> ---
>>  qemu/hw/device-assignment.c |   28 ++++++++++++++++++++++++++--
>>  qemu/hw/device-assignment.h |    1 +
>>  2 files changed, 27 insertions(+), 2 deletions(-)


[-- Attachment #2: 0003-kvm-qemu-free-device-on-error-in-init_assigned_dev-v2.patch --]
[-- Type: application/octet-stream, Size: 3350 bytes --]

From ce48b0d6c636d8f49bc5977d1d144fa047273846 Mon Sep 17 00:00:00 2001
From: Weidong Han <weidong.han@intel.com>
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 <markmc@redhat.com>
Signed-off-by: Weidong Han <weidong.han@intel.com>
---
 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;
     }
 
-- 
1.6.0.4


      reply	other threads:[~2009-02-19  2:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-18  7:13 [PATCH 7/8] kvm: qemu: deassign device from guest Han, Weidong
2009-02-18 17:18 ` Marcelo Tosatti
2009-02-19  2:49   ` Han, Weidong [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=715D42877B251141A38726ABF5CABF2C0195A1FC47@pdsmsx503.ccr.corp.intel.com \
    --to=weidong.han@intel.com \
    --cc=avi@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=markmc@redhat.com \
    --cc=mtosatti@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.