All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [FOR 0.12 PATCH 0/4] misc bugfixes
@ 2009-12-10 10:11 Gerd Hoffmann
  2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 1/4] pci: don't abort() when trying to hotplug with acpi off Gerd Hoffmann
                   ` (4 more replies)
  0 siblings, 5 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2009-12-10 10:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

  Hi,

A bunch of fixes for various bugs, mostly hotplug stuff found by Daniel.

please apply,
  Gerd

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] [FOR 0.12 PATCH 1/4] pci: don't abort() when trying to hotplug with acpi off.
  2009-12-10 10:11 [Qemu-devel] [FOR 0.12 PATCH 0/4] misc bugfixes Gerd Hoffmann
@ 2009-12-10 10:11 ` Gerd Hoffmann
  2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available Gerd Hoffmann
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2009-12-10 10:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

The PCI bus on x86 requires ACPI for hotplug support, thus disbling ACPI
also disables hotplug for the PCI bus.  This patch makes qemu check
whenever the PCI bus in question can handle hotplug before trying to add
devices.  This is needed because qdev will abort() on any attempt to
hotplug devices into a non-hotpluggable bus.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci-hotplug.c |   17 ++++++++++++++++-
 1 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 081d6d1..7e5c51d 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -40,7 +40,18 @@ static PCIDevice *qemu_pci_hot_add_nic(Monitor *mon,
                                        const char *opts_str)
 {
     QemuOpts *opts;
-    int ret;
+    PCIBus *bus;
+    int ret, devfn;
+
+    bus = pci_get_bus_devfn(&devfn, devaddr);
+    if (!bus) {
+        monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
+        return NULL;
+    }
+    if (!((BusState*)bus)->allow_hotplug) {
+        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
+        return NULL;
+    }
 
     opts = qemu_opts_parse(&qemu_net_opts, opts_str ? opts_str : "", NULL);
     if (!opts) {
@@ -179,6 +190,10 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
         monitor_printf(mon, "Invalid PCI device address %s\n", devaddr);
         return NULL;
     }
+    if (!((BusState*)bus)->allow_hotplug) {
+        monitor_printf(mon, "PCI bus doesn't support hotplug\n");
+        return NULL;
+    }
 
     switch (type) {
     case IF_SCSI:
-- 
1.6.5.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available.
  2009-12-10 10:11 [Qemu-devel] [FOR 0.12 PATCH 0/4] misc bugfixes Gerd Hoffmann
  2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 1/4] pci: don't abort() when trying to hotplug with acpi off Gerd Hoffmann
@ 2009-12-10 10:11 ` Gerd Hoffmann
  2009-12-10 12:08   ` [Qemu-devel] " Michael S. Tsirkin
  2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 3/4] scsi: fix drive hotplug Gerd Hoffmann
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 17+ messages in thread
From: Gerd Hoffmann @ 2009-12-10 10:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Current PCI code will simply hw_error() and thus abort in case no free
PCI slot is available or the requested PCI slot is already in use by
another device.  For the hotplug case this behavior is not acceptable.
This patch makes qemu pass up the error properly, so the calling code
can decide whenever it wants to exit with an error (on startup) or
whenever it wants to continue (hotplug).

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 4f662b7..404eead 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -580,11 +580,13 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
             if (!bus->devices[devfn])
                 goto found;
         }
-        hw_error("PCI: no devfn available for %s, all in use\n", name);
+        qemu_error("PCI: no devfn available for %s, all in use\n", name);
+        return NULL;
     found: ;
     } else if (bus->devices[devfn]) {
-        hw_error("PCI: devfn %d not available for %s, in use by %s\n", devfn,
+        qemu_error("PCI: devfn %d not available for %s, in use by %s\n", devfn,
                  name, bus->devices[devfn]->name);
+        return NULL;
     }
     pci_dev->bus = bus;
     pci_dev->devfn = devfn;
@@ -625,6 +627,9 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
     pci_dev = do_pci_register_device(pci_dev, bus, name, devfn,
                                      config_read, config_write,
                                      PCI_HEADER_TYPE_NORMAL);
+    if (pci_dev == NULL) {
+        hw_error("PCI: can't register device\n");
+    }
     return pci_dev;
 }
 static target_phys_addr_t pci_to_cpu_addr(target_phys_addr_t addr)
@@ -1376,6 +1381,8 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
     pci_dev = do_pci_register_device(pci_dev, bus, base->name, devfn,
                                      info->config_read, info->config_write,
                                      info->header_type);
+    if (pci_dev == NULL)
+        return -1;
     rc = info->init(pci_dev);
     if (rc != 0)
         return rc;
-- 
1.6.5.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [FOR 0.12 PATCH 3/4] scsi: fix drive hotplug.
  2009-12-10 10:11 [Qemu-devel] [FOR 0.12 PATCH 0/4] misc bugfixes Gerd Hoffmann
  2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 1/4] pci: don't abort() when trying to hotplug with acpi off Gerd Hoffmann
  2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available Gerd Hoffmann
@ 2009-12-10 10:11 ` Gerd Hoffmann
  2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 4/4] QemuOpts: allow larger option values Gerd Hoffmann
  2009-12-10 12:03 ` [Qemu-devel] Re: [FOR 0.12 PATCH 0/4] misc bugfixes Michael S. Tsirkin
  4 siblings, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2009-12-10 10:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

This patch fills the DriveInfo->unit after hotplugging a scsi disk.
It makes a difference when auto-assigning a scsi id, where unit was
left filled with '-1' instead of the actual scsi id.

With this patch applied the the drive naming logic in drive_init() works
as good as it did in previous releases.  Which means it works fine with
a single scsi bus.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/pci-hotplug.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index 7e5c51d..9e8e6ed 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -93,6 +93,7 @@ static int scsi_hot_add(DeviceState *adapter, DriveInfo *dinfo, int printinfo)
      */
     dinfo->unit = qemu_opt_get_number(dinfo->opts, "unit", -1);
     scsidev = scsi_bus_legacy_add_drive(scsibus, dinfo, dinfo->unit);
+    dinfo->unit = scsidev->id;
 
     if (printinfo)
         qemu_error("OK bus %d, unit %d\n", scsibus->busnr, scsidev->id);
-- 
1.6.5.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] [FOR 0.12 PATCH 4/4] QemuOpts: allow larger option values.
  2009-12-10 10:11 [Qemu-devel] [FOR 0.12 PATCH 0/4] misc bugfixes Gerd Hoffmann
                   ` (2 preceding siblings ...)
  2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 3/4] scsi: fix drive hotplug Gerd Hoffmann
@ 2009-12-10 10:11 ` Gerd Hoffmann
  2009-12-10 12:03 ` [Qemu-devel] Re: [FOR 0.12 PATCH 0/4] misc bugfixes Michael S. Tsirkin
  4 siblings, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2009-12-10 10:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: Gerd Hoffmann

Use case: loooooooooooooooooong file names for -drive file=...

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 qemu-option.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/qemu-option.c b/qemu-option.c
index b009109..24392fc 100644
--- a/qemu-option.c
+++ b/qemu-option.c
@@ -705,7 +705,7 @@ int qemu_opts_print(QemuOpts *opts, void *dummy)
 
 int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname)
 {
-    char option[128], value[128];
+    char option[128], value[1024];
     const char *p,*pe,*pc;
 
     for (p = params; *p != '\0'; p++) {
@@ -751,7 +751,7 @@ int qemu_opts_do_parse(QemuOpts *opts, const char *params, const char *firstname
 
 QemuOpts *qemu_opts_parse(QemuOptsList *list, const char *params, const char *firstname)
 {
-    char value[128], *id = NULL;
+    char value[1024], *id = NULL;
     const char *p;
     QemuOpts *opts;
 
-- 
1.6.5.2

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* [Qemu-devel] Re: [FOR 0.12 PATCH 0/4] misc bugfixes
  2009-12-10 10:11 [Qemu-devel] [FOR 0.12 PATCH 0/4] misc bugfixes Gerd Hoffmann
                   ` (3 preceding siblings ...)
  2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 4/4] QemuOpts: allow larger option values Gerd Hoffmann
@ 2009-12-10 12:03 ` Michael S. Tsirkin
  2009-12-10 12:15   ` Gerd Hoffmann
  2009-12-10 15:37   ` Anthony Liguori
  4 siblings, 2 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2009-12-10 12:03 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Thu, Dec 10, 2009 at 11:11:04AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> A bunch of fixes for various bugs, mostly hotplug stuff found by Daniel.
> 
> please apply,
>   Gerd
> 

I am kind of confused by the FOR 0.12 prefix - these
patches are not on master yet, are they?
Should patches land on master before being backported
to 0.12?

-- 
MST

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] Re: [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available.
  2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available Gerd Hoffmann
@ 2009-12-10 12:08   ` Michael S. Tsirkin
  2009-12-10 12:19     ` Gerd Hoffmann
  2009-12-10 12:33     ` Alexander Graf
  0 siblings, 2 replies; 17+ messages in thread
From: Michael S. Tsirkin @ 2009-12-10 12:08 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Thu, Dec 10, 2009 at 11:11:06AM +0100, Gerd Hoffmann wrote:
> Current PCI code will simply hw_error() and thus abort in case no free
> PCI slot is available or the requested PCI slot is already in use by
> another device.  For the hotplug case this behavior is not acceptable.
> This patch makes qemu pass up the error properly, so the calling code
> can decide whenever it wants to exit with an error (on startup) or
> whenever it wants to continue (hotplug).
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>


Good stuff. However

> ---
>  hw/pci.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci.c b/hw/pci.c
> index 4f662b7..404eead 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -580,11 +580,13 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>              if (!bus->devices[devfn])
>                  goto found;
>          }
> -        hw_error("PCI: no devfn available for %s, all in use\n", name);
> +        qemu_error("PCI: no devfn available for %s, all in use\n", name);
> +        return NULL;
>      found: ;
>      } else if (bus->devices[devfn]) {
> -        hw_error("PCI: devfn %d not available for %s, in use by %s\n", devfn,
> +        qemu_error("PCI: devfn %d not available for %s, in use by %s\n", devfn,
>                   name, bus->devices[devfn]->name);
> +        return NULL;
>      }
>      pci_dev->bus = bus;
>      pci_dev->devfn = devfn;
> @@ -625,6 +627,9 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
>      pci_dev = do_pci_register_device(pci_dev, bus, name, devfn,
>                                       config_read, config_write,
>                                       PCI_HEADER_TYPE_NORMAL);
> +    if (pci_dev == NULL) {
> +        hw_error("PCI: can't register device\n");
> +    }

Can you please use !pci_dev for these checks?

>      return pci_dev;
>  }
>  static target_phys_addr_t pci_to_cpu_addr(target_phys_addr_t addr)
> @@ -1376,6 +1381,8 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
>      pci_dev = do_pci_register_device(pci_dev, bus, base->name, devfn,
>                                       info->config_read, info->config_write,
>                                       info->header_type);
> +    if (pci_dev == NULL)
> +        return -1;

And here too.

>      rc = info->init(pci_dev);
>      if (rc != 0)
>          return rc;
> -- 
> 1.6.5.2
> 
> 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] Re: [FOR 0.12 PATCH 0/4] misc bugfixes
  2009-12-10 12:03 ` [Qemu-devel] Re: [FOR 0.12 PATCH 0/4] misc bugfixes Michael S. Tsirkin
@ 2009-12-10 12:15   ` Gerd Hoffmann
  2009-12-10 15:37   ` Anthony Liguori
  1 sibling, 0 replies; 17+ messages in thread
From: Gerd Hoffmann @ 2009-12-10 12:15 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel

On 12/10/09 13:03, Michael S. Tsirkin wrote:
> On Thu, Dec 10, 2009 at 11:11:04AM +0100, Gerd Hoffmann wrote:
>>    Hi,
>>
>> A bunch of fixes for various bugs, mostly hotplug stuff found by Daniel.
>>
>> please apply,
>>    Gerd
>>
>
> I am kind of confused by the FOR 0.12 prefix - these
> patches are not on master yet, are they?

No.

> Should patches land on master before being backported
> to 0.12?

Anthony asked to tag patches this way if they should be merged for the 
0.12 release.  So, after branching, they should go to both master and 
0.12 I think.  Do we have a 0.12 branch already?

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] Re: [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available.
  2009-12-10 12:08   ` [Qemu-devel] " Michael S. Tsirkin
@ 2009-12-10 12:19     ` Gerd Hoffmann
  2009-12-10 12:23       ` Michael S. Tsirkin
  2009-12-10 12:33     ` Alexander Graf
  1 sibling, 1 reply; 17+ messages in thread
From: Gerd Hoffmann @ 2009-12-10 12:19 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: qemu-devel

   Hi,

>> +    if (pci_dev == NULL) {
>> +        hw_error("PCI: can't register device\n");
>> +    }
>
> Can you please use !pci_dev for these checks?

Why?  IMHO the code is more readable that way.  It is easy to miss a 
single '!' character when reading the code, so I tend to write such 
tests in a more verbose fashion.

cheers,
   Gerd

^ permalink raw reply	[flat|nested] 17+ messages in thread

* [Qemu-devel] Re: [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available.
  2009-12-10 12:19     ` Gerd Hoffmann
@ 2009-12-10 12:23       ` Michael S. Tsirkin
  2009-12-10 13:22         ` Gleb Natapov
  0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2009-12-10 12:23 UTC (permalink / raw)
  To: Gerd Hoffmann; +Cc: qemu-devel

On Thu, Dec 10, 2009 at 01:19:10PM +0100, Gerd Hoffmann wrote:
>   Hi,
>
>>> +    if (pci_dev == NULL) {
>>> +        hw_error("PCI: can't register device\n");
>>> +    }
>>
>> Can you please use !pci_dev for these checks?
>
> Why?  IMHO the code is more readable that way.  It is easy to miss a  
> single '!' character when reading the code, so I tend to write such  
> tests in a more verbose fashion.
>
> cheers,
>   Gerd

Reader has limited short term memory. Don't fill it up with
irrelevant detail. In places where it might be confusing,
a comment might be appropriate, this is not one of them.

C is a terse language. !x is a standard idiom. Let's use it.

-- 
MST

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Re: [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available.
  2009-12-10 12:08   ` [Qemu-devel] " Michael S. Tsirkin
  2009-12-10 12:19     ` Gerd Hoffmann
@ 2009-12-10 12:33     ` Alexander Graf
  1 sibling, 0 replies; 17+ messages in thread
From: Alexander Graf @ 2009-12-10 12:33 UTC (permalink / raw)
  To: Michael S.Tsirkin; +Cc: Gerd Hoffmann, qemu-devel


On 10.12.2009, at 13:08, Michael S. Tsirkin wrote:

> On Thu, Dec 10, 2009 at 11:11:06AM +0100, Gerd Hoffmann wrote:
>> Current PCI code will simply hw_error() and thus abort in case no free
>> PCI slot is available or the requested PCI slot is already in use by
>> another device.  For the hotplug case this behavior is not acceptable.
>> This patch makes qemu pass up the error properly, so the calling code
>> can decide whenever it wants to exit with an error (on startup) or
>> whenever it wants to continue (hotplug).
>> 
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> 
> Good stuff. However
> 
>> ---
>> hw/pci.c |   11 +++++++++--
>> 1 files changed, 9 insertions(+), 2 deletions(-)
>> 
>> diff --git a/hw/pci.c b/hw/pci.c
>> index 4f662b7..404eead 100644
>> --- a/hw/pci.c
>> +++ b/hw/pci.c
>> @@ -580,11 +580,13 @@ static PCIDevice *do_pci_register_device(PCIDevice *pci_dev, PCIBus *bus,
>>             if (!bus->devices[devfn])
>>                 goto found;
>>         }
>> -        hw_error("PCI: no devfn available for %s, all in use\n", name);
>> +        qemu_error("PCI: no devfn available for %s, all in use\n", name);
>> +        return NULL;
>>     found: ;
>>     } else if (bus->devices[devfn]) {
>> -        hw_error("PCI: devfn %d not available for %s, in use by %s\n", devfn,
>> +        qemu_error("PCI: devfn %d not available for %s, in use by %s\n", devfn,
>>                  name, bus->devices[devfn]->name);
>> +        return NULL;
>>     }
>>     pci_dev->bus = bus;
>>     pci_dev->devfn = devfn;
>> @@ -625,6 +627,9 @@ PCIDevice *pci_register_device(PCIBus *bus, const char *name,
>>     pci_dev = do_pci_register_device(pci_dev, bus, name, devfn,
>>                                      config_read, config_write,
>>                                      PCI_HEADER_TYPE_NORMAL);
>> +    if (pci_dev == NULL) {
>> +        hw_error("PCI: can't register device\n");
>> +    }
> 
> Can you please use !pci_dev for these checks?
> 
>>     return pci_dev;
>> }
>> static target_phys_addr_t pci_to_cpu_addr(target_phys_addr_t addr)
>> @@ -1376,6 +1381,8 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base)
>>     pci_dev = do_pci_register_device(pci_dev, bus, base->name, devfn,
>>                                      info->config_read, info->config_write,
>>                                      info->header_type);
>> +    if (pci_dev == NULL)
>> +        return -1;
> 
> And here too.

OMG! Broken coding style!

:)

Alex

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Re: [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available.
  2009-12-10 12:23       ` Michael S. Tsirkin
@ 2009-12-10 13:22         ` Gleb Natapov
  2009-12-10 18:04           ` Michael S. Tsirkin
  0 siblings, 1 reply; 17+ messages in thread
From: Gleb Natapov @ 2009-12-10 13:22 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Gerd Hoffmann, qemu-devel

On Thu, Dec 10, 2009 at 02:23:05PM +0200, Michael S. Tsirkin wrote:
> On Thu, Dec 10, 2009 at 01:19:10PM +0100, Gerd Hoffmann wrote:
> >   Hi,
> >
> >>> +    if (pci_dev == NULL) {
> >>> +        hw_error("PCI: can't register device\n");
> >>> +    }
> >>
> >> Can you please use !pci_dev for these checks?
> >
> > Why?  IMHO the code is more readable that way.  It is easy to miss a  
> > single '!' character when reading the code, so I tend to write such  
> > tests in a more verbose fashion.
> >
> > cheers,
> >   Gerd
> 
> Reader has limited short term memory. Don't fill it up with
> irrelevant detail. In places where it might be confusing,
> a comment might be appropriate, this is not one of them.
> 
> C is a terse language. !x is a standard idiom. Let's use it.
> 
! is for boolean. pci_dev is not boolean.

--
			Gleb.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Re: [FOR 0.12 PATCH 0/4] misc bugfixes
  2009-12-10 12:03 ` [Qemu-devel] Re: [FOR 0.12 PATCH 0/4] misc bugfixes Michael S. Tsirkin
  2009-12-10 12:15   ` Gerd Hoffmann
@ 2009-12-10 15:37   ` Anthony Liguori
  1 sibling, 0 replies; 17+ messages in thread
From: Anthony Liguori @ 2009-12-10 15:37 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Gerd Hoffmann, qemu-devel

Michael S. Tsirkin wrote:
> On Thu, Dec 10, 2009 at 11:11:04AM +0100, Gerd Hoffmann wrote:
>   
>>   Hi,
>>
>> A bunch of fixes for various bugs, mostly hotplug stuff found by Daniel.
>>
>> please apply,
>>   Gerd
>>
>>     
>
> I am kind of confused by the FOR 0.12 prefix - these
> patches are not on master yet, are they?
> Should patches land on master before being backported
> to 0.12?
>   

Right now, there's almost no difference between master and stable-0.12.  
I always commit to master first and then cherry pick.

FOR 0.12 helps me know that I need to cherry pick from master into 
stable-0.12.

Regards,

Anthony Liguori

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Re: [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available.
  2009-12-10 13:22         ` Gleb Natapov
@ 2009-12-10 18:04           ` Michael S. Tsirkin
  2009-12-10 19:13             ` Gleb Natapov
  0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2009-12-10 18:04 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: Gerd Hoffmann, qemu-devel

On Thu, Dec 10, 2009 at 03:22:52PM +0200, Gleb Natapov wrote:
> On Thu, Dec 10, 2009 at 02:23:05PM +0200, Michael S. Tsirkin wrote:
> > On Thu, Dec 10, 2009 at 01:19:10PM +0100, Gerd Hoffmann wrote:
> > >   Hi,
> > >
> > >>> +    if (pci_dev == NULL) {
> > >>> +        hw_error("PCI: can't register device\n");
> > >>> +    }
> > >>
> > >> Can you please use !pci_dev for these checks?
> > >
> > > Why?  IMHO the code is more readable that way.  It is easy to miss a  
> > > single '!' character when reading the code, so I tend to write such  
> > > tests in a more verbose fashion.
> > >
> > > cheers,
> > >   Gerd
> > 
> > Reader has limited short term memory. Don't fill it up with
> > irrelevant detail. In places where it might be confusing,
> > a comment might be appropriate, this is not one of them.
> > 
> > C is a terse language. !x is a standard idiom. Let's use it.
> > 
> ! is for boolean.

In which language? Not in C.

> pci_dev is not boolean.
> --
> 			Gleb.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Re: [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available.
  2009-12-10 18:04           ` Michael S. Tsirkin
@ 2009-12-10 19:13             ` Gleb Natapov
  2009-12-11 10:37               ` Michael S. Tsirkin
  0 siblings, 1 reply; 17+ messages in thread
From: Gleb Natapov @ 2009-12-10 19:13 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Gerd Hoffmann, qemu-devel

On Thu, Dec 10, 2009 at 08:04:56PM +0200, Michael S. Tsirkin wrote:
> On Thu, Dec 10, 2009 at 03:22:52PM +0200, Gleb Natapov wrote:
> > On Thu, Dec 10, 2009 at 02:23:05PM +0200, Michael S. Tsirkin wrote:
> > > On Thu, Dec 10, 2009 at 01:19:10PM +0100, Gerd Hoffmann wrote:
> > > >   Hi,
> > > >
> > > >>> +    if (pci_dev == NULL) {
> > > >>> +        hw_error("PCI: can't register device\n");
> > > >>> +    }
> > > >>
> > > >> Can you please use !pci_dev for these checks?
> > > >
> > > > Why?  IMHO the code is more readable that way.  It is easy to miss a  
> > > > single '!' character when reading the code, so I tend to write such  
> > > > tests in a more verbose fashion.
> > > >
> > > > cheers,
> > > >   Gerd
> > > 
> > > Reader has limited short term memory. Don't fill it up with
> > > irrelevant detail. In places where it might be confusing,
> > > a comment might be appropriate, this is not one of them.
> > > 
> > > C is a terse language. !x is a standard idiom. Let's use it.
> > > 
> > ! is for boolean.
> 
> In which language? Not in C.
> 
In boolean. My point is no need to force your style on everyone if it is
not in coding style document. == often much more readable then !.

> > pci_dev is not boolean.
> > --
> > 			Gleb.

--
			Gleb.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Re: [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available.
  2009-12-10 19:13             ` Gleb Natapov
@ 2009-12-11 10:37               ` Michael S. Tsirkin
  2009-12-11 11:03                 ` Gleb Natapov
  0 siblings, 1 reply; 17+ messages in thread
From: Michael S. Tsirkin @ 2009-12-11 10:37 UTC (permalink / raw)
  To: Gleb Natapov; +Cc: Gerd Hoffmann, qemu-devel

On Thu, Dec 10, 2009 at 09:13:06PM +0200, Gleb Natapov wrote:
> On Thu, Dec 10, 2009 at 08:04:56PM +0200, Michael S. Tsirkin wrote:
> > On Thu, Dec 10, 2009 at 03:22:52PM +0200, Gleb Natapov wrote:
> > > On Thu, Dec 10, 2009 at 02:23:05PM +0200, Michael S. Tsirkin wrote:
> > > > On Thu, Dec 10, 2009 at 01:19:10PM +0100, Gerd Hoffmann wrote:
> > > > >   Hi,
> > > > >
> > > > >>> +    if (pci_dev == NULL) {
> > > > >>> +        hw_error("PCI: can't register device\n");
> > > > >>> +    }
> > > > >>
> > > > >> Can you please use !pci_dev for these checks?
> > > > >
> > > > > Why?  IMHO the code is more readable that way.  It is easy to miss a  
> > > > > single '!' character when reading the code, so I tend to write such  
> > > > > tests in a more verbose fashion.
> > > > >
> > > > > cheers,
> > > > >   Gerd
> > > > 
> > > > Reader has limited short term memory. Don't fill it up with
> > > > irrelevant detail. In places where it might be confusing,
> > > > a comment might be appropriate, this is not one of them.
> > > > 
> > > > C is a terse language. !x is a standard idiom. Let's use it.
> > > > 
> > > ! is for boolean.
> > 
> > In which language? Not in C.
> > 
> In boolean. My point is no need to force your style on everyone

I am not forcing style on anyone - how can I do this?  I hacked on pci
and by now there's no == NULL in pci.c anywhere ;). I will be
happier if none is added now.

> if it is not in coding style document.

Heh, we don't need to document everything.

> == often much more readable then !.

Matter of taste probably.

> > > pci_dev is not boolean.
> > > --
> > > 			Gleb.
> 
> --
> 			Gleb.

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [Qemu-devel] Re: [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available.
  2009-12-11 10:37               ` Michael S. Tsirkin
@ 2009-12-11 11:03                 ` Gleb Natapov
  0 siblings, 0 replies; 17+ messages in thread
From: Gleb Natapov @ 2009-12-11 11:03 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Gerd Hoffmann, qemu-devel

On Fri, Dec 11, 2009 at 12:37:31PM +0200, Michael S. Tsirkin wrote:
> On Thu, Dec 10, 2009 at 09:13:06PM +0200, Gleb Natapov wrote:
> > On Thu, Dec 10, 2009 at 08:04:56PM +0200, Michael S. Tsirkin wrote:
> > > On Thu, Dec 10, 2009 at 03:22:52PM +0200, Gleb Natapov wrote:
> > > > On Thu, Dec 10, 2009 at 02:23:05PM +0200, Michael S. Tsirkin wrote:
> > > > > On Thu, Dec 10, 2009 at 01:19:10PM +0100, Gerd Hoffmann wrote:
> > > > > >   Hi,
> > > > > >
> > > > > >>> +    if (pci_dev == NULL) {
> > > > > >>> +        hw_error("PCI: can't register device\n");
> > > > > >>> +    }
> > > > > >>
> > > > > >> Can you please use !pci_dev for these checks?
> > > > > >
> > > > > > Why?  IMHO the code is more readable that way.  It is easy to miss a  
> > > > > > single '!' character when reading the code, so I tend to write such  
> > > > > > tests in a more verbose fashion.
> > > > > >
> > > > > > cheers,
> > > > > >   Gerd
> > > > > 
> > > > > Reader has limited short term memory. Don't fill it up with
> > > > > irrelevant detail. In places where it might be confusing,
> > > > > a comment might be appropriate, this is not one of them.
> > > > > 
> > > > > C is a terse language. !x is a standard idiom. Let's use it.
> > > > > 
> > > > ! is for boolean.
> > > 
> > > In which language? Not in C.
> > > 
> > In boolean. My point is no need to force your style on everyone
> 
> I am not forcing style on anyone - how can I do this?  I hacked on pci
> and by now there's no == NULL in pci.c anywhere ;). I will be
> happier if none is added now.
> 
> > if it is not in coding style document.
> 
> Heh, we don't need to document everything.
> 
> > == often much more readable then !.
> 
> Matter of taste probably.
> 
Exactly! (Or should I say "Exactly=="?)

--
			Gleb.

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2009-12-11 11:03 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-10 10:11 [Qemu-devel] [FOR 0.12 PATCH 0/4] misc bugfixes Gerd Hoffmann
2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 1/4] pci: don't abort() when trying to hotplug with acpi off Gerd Hoffmann
2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 2/4] pci: don't hw_error() when no slot is available Gerd Hoffmann
2009-12-10 12:08   ` [Qemu-devel] " Michael S. Tsirkin
2009-12-10 12:19     ` Gerd Hoffmann
2009-12-10 12:23       ` Michael S. Tsirkin
2009-12-10 13:22         ` Gleb Natapov
2009-12-10 18:04           ` Michael S. Tsirkin
2009-12-10 19:13             ` Gleb Natapov
2009-12-11 10:37               ` Michael S. Tsirkin
2009-12-11 11:03                 ` Gleb Natapov
2009-12-10 12:33     ` Alexander Graf
2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 3/4] scsi: fix drive hotplug Gerd Hoffmann
2009-12-10 10:11 ` [Qemu-devel] [FOR 0.12 PATCH 4/4] QemuOpts: allow larger option values Gerd Hoffmann
2009-12-10 12:03 ` [Qemu-devel] Re: [FOR 0.12 PATCH 0/4] misc bugfixes Michael S. Tsirkin
2009-12-10 12:15   ` Gerd Hoffmann
2009-12-10 15:37   ` Anthony Liguori

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.