[Qemu-devel] [PATCH v5 0/4] Convert to realize and fix error handling

[Qemu-devel] [PATCH v5 0/4] Convert to realize and fix error handling

[Mao Zhongyi]

v5:
* Patch 1 removed the dead error handling that was previously missing.
* Patch 2 and 3 has not changed.
* Patch 4 is a new patch to fix the unusual macro name.

v4:
* Patch 1 is following Markus's suggestion that remove the dead error
  handling.
* Patch 2 is separate from patch 1 to plug the memory leak in the v3.
* Patch 3 is based on the patch 1 in the v3. Meanwhile, dorp the superfluous
  prefix "rocker:" and adjust the commit message.

v3:
* Following Jason's suggstion that add suitable error message to 
  each error site.
* Modified the commit message to make it easier to read.

v2:
* Following Philippe's suggestion that shorten the patch subject 
  "hw/net/rocker/rocker" to "net/rocker". 
* Use a consistent log format to report error message.
* Add a specific goto label "err_name_too_long" to make a correct
  cleanup.

Mao Zhongyi (4):
  net/rocker: Remove the dead error handling
  net/rocker: Plug memory leak in pci_rocker_init()
  net/rocker: Convert to realize()
  net/rocker: Fix the unusual macro name

 hw/net/rocker/rocker.c        | 93 ++++++++++---------------------------------
 hw/net/rocker/rocker_desc.c   | 10 -----
 hw/net/rocker/rocker_fp.c     |  4 --
 hw/net/rocker/rocker_of_dpa.c | 20 ----------
 hw/net/rocker/rocker_world.c  | 12 +++---
 5 files changed, 27 insertions(+), 112 deletions(-)

-- 
2.9.3

[Qemu-devel] [PATCH v5 1/4] net/rocker: Remove the dead error handling

[Mao Zhongyi]

Memory allocation functions like world_alloc, desc_ring_alloc etc,
they are all wrappers around g_malloc, g_new etc. But g_malloc and 
similar functions doesn't return null. Because they ignore the fact 
that g_malloc() of 0 bytes returns null. So error checks for these
allocation failure are superfluous. Now, remove them entirely.

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
---
 hw/net/rocker/rocker.c        | 46 -------------------------------------------
 hw/net/rocker/rocker_desc.c   | 10 ----------
 hw/net/rocker/rocker_fp.c     |  4 ----
 hw/net/rocker/rocker_of_dpa.c | 20 -------------------
 hw/net/rocker/rocker_world.c  | 12 +++++------
 5 files changed, 5 insertions(+), 87 deletions(-)

diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 6e70fdd..d01ba9d 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -239,10 +239,6 @@ static int tx_consume(Rocker *r, DescInfo *info)
         }
         iov[iovcnt].iov_len = frag_len;
         iov[iovcnt].iov_base = g_malloc(frag_len);
-        if (!iov[iovcnt].iov_base) {
-            err = -ROCKER_ENOMEM;
-            goto err_no_mem;
-        }
 
         if (pci_dma_read(dev, frag_addr, iov[iovcnt].iov_base,
                      iov[iovcnt].iov_len)) {
@@ -262,7 +258,6 @@ static int tx_consume(Rocker *r, DescInfo *info)
 
 err_too_many_frags:
 err_bad_io:
-err_no_mem:
 err_bad_attr:
     for (i = 0; i < ROCKER_TX_FRAGS_MAX; i++) {
         g_free(iov[i].iov_base);
@@ -674,10 +669,6 @@ int rx_produce(World *world, uint32_t pport,
      */
 
     data = g_malloc(data_size);
-    if (!data) {
-        err = -ROCKER_ENOMEM;
-        goto out;
-    }
     iov_to_buf(iov, iovcnt, 0, data, data_size);
     pci_dma_write(dev, frag_addr, data, data_size);
     g_free(data);
@@ -722,11 +713,6 @@ static void rocker_test_dma_ctrl(Rocker *r, uint32_t val)
 
     buf = g_malloc(r->test_dma_size);
 
-    if (!buf) {
-        DPRINTF("test dma buffer alloc failed");
-        return;
-    }
-
     switch (val) {
     case ROCKER_TEST_DMA_CTRL_CLEAR:
         memset(buf, 0, r->test_dma_size);
@@ -1313,13 +1299,6 @@ static int pci_rocker_init(PCIDevice *dev)
 
     r->worlds[ROCKER_WORLD_TYPE_OF_DPA] = of_dpa_world_alloc(r);
 
-    for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
-        if (!r->worlds[i]) {
-            err = -ENOMEM;
-            goto err_world_alloc;
-        }
-    }
-
     if (!r->world_name) {
         r->world_name = g_strdup(world_name(r->worlds[ROCKER_WORLD_TYPE_OF_DPA]));
     }
@@ -1396,9 +1375,6 @@ static int pci_rocker_init(PCIDevice *dev)
     }
 
     r->rings = g_new(DescRing *, rocker_pci_ring_count(r));
-    if (!r->rings) {
-        goto err_rings_alloc;
-    }
 
     /* Rings are ordered like this:
      * - command ring
@@ -1410,14 +1386,9 @@ static int pci_rocker_init(PCIDevice *dev)
      * .....
      */
 
-    err = -ENOMEM;
     for (i = 0; i < rocker_pci_ring_count(r); i++) {
         DescRing *ring = desc_ring_alloc(r, i);
 
-        if (!ring) {
-            goto err_ring_alloc;
-        }
-
         if (i == ROCKER_RING_CMD) {
             desc_ring_set_consume(ring, cmd_consume, ROCKER_MSIX_VEC_CMD);
         } else if (i == ROCKER_RING_EVENT) {
@@ -1437,10 +1408,6 @@ static int pci_rocker_init(PCIDevice *dev)
             fp_port_alloc(r, r->name, &r->fp_start_macaddr,
                           i, &r->fp_ports_peers[i]);
 
-        if (!port) {
-            goto err_port_alloc;
-        }
-
         r->fp_port[i] = port;
         fp_port_set_world(port, r->world_dflt);
     }
@@ -1449,25 +1416,12 @@ static int pci_rocker_init(PCIDevice *dev)
 
     return 0;
 
-err_port_alloc:
-    for (--i; i >= 0; i--) {
-        FpPort *port = r->fp_port[i];
-        fp_port_free(port);
-    }
-    i = rocker_pci_ring_count(r);
-err_ring_alloc:
-    for (--i; i >= 0; i--) {
-        desc_ring_free(r->rings[i]);
-    }
-    g_free(r->rings);
-err_rings_alloc:
 err_duplicate:
     rocker_msix_uninit(r);
 err_msix_init:
     object_unparent(OBJECT(&r->msix_bar));
     object_unparent(OBJECT(&r->mmio));
 err_world_type_by_name:
-err_world_alloc:
     for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
         if (r->worlds[i]) {
             world_free(r->worlds[i]);
diff --git a/hw/net/rocker/rocker_desc.c b/hw/net/rocker/rocker_desc.c
index ac02797..d0df89a 100644
--- a/hw/net/rocker/rocker_desc.c
+++ b/hw/net/rocker/rocker_desc.c
@@ -65,10 +65,6 @@ char *desc_get_buf(DescInfo *info, bool read_only)
         info->buf_size = size;
     }
 
-    if (!info->buf) {
-        return NULL;
-    }
-
     if (pci_dma_read(dev, le64_to_cpu(info->desc.buf_addr), info->buf, size)) {
         return NULL;
     }
@@ -144,9 +140,6 @@ bool desc_ring_set_size(DescRing *ring, uint32_t size)
     ring->head = ring->tail = 0;
 
     ring->info = g_renew(DescInfo, ring->info, size);
-    if (!ring->info) {
-        return false;
-    }
 
     memset(ring->info, 0, size * sizeof(DescInfo));
 
@@ -347,9 +340,6 @@ DescRing *desc_ring_alloc(Rocker *r, int index)
     DescRing *ring;
 
     ring = g_new0(DescRing, 1);
-    if (!ring) {
-        return NULL;
-    }
 
     ring->r = r;
     ring->index = index;
diff --git a/hw/net/rocker/rocker_fp.c b/hw/net/rocker/rocker_fp.c
index 1305ac3..4b3c984 100644
--- a/hw/net/rocker/rocker_fp.c
+++ b/hw/net/rocker/rocker_fp.c
@@ -226,10 +226,6 @@ FpPort *fp_port_alloc(Rocker *r, char *sw_name,
 {
     FpPort *port = g_new0(FpPort, 1);
 
-    if (!port) {
-        return NULL;
-    }
-
     port->r = r;
     port->index = index;
     port->pport = index + 1;
diff --git a/hw/net/rocker/rocker_of_dpa.c b/hw/net/rocker/rocker_of_dpa.c
index 9b1e0d2..191a58e 100644
--- a/hw/net/rocker/rocker_of_dpa.c
+++ b/hw/net/rocker/rocker_of_dpa.c
@@ -368,9 +368,6 @@ static OfDpaFlow *of_dpa_flow_alloc(uint64_t cookie)
     int64_t now = qemu_clock_get_ms(QEMU_CLOCK_VIRTUAL) / 1000;
 
     flow = g_new0(OfDpaFlow, 1);
-    if (!flow) {
-        return NULL;
-    }
 
     flow->cookie = cookie;
     flow->mask.tbl_id = 0xffffffff;
@@ -813,10 +810,6 @@ static OfDpaGroup *of_dpa_group_alloc(uint32_t id)
 {
     OfDpaGroup *group = g_new0(OfDpaGroup, 1);
 
-    if (!group) {
-        return NULL;
-    }
-
     group->id = id;
 
     return group;
@@ -1867,9 +1860,6 @@ static int of_dpa_cmd_flow_add(OfDpa *of_dpa, uint64_t cookie,
     }
 
     flow = of_dpa_flow_alloc(cookie);
-    if (!flow) {
-        return -ROCKER_ENOMEM;
-    }
 
     err = of_dpa_cmd_flow_add_mod(of_dpa, flow, flow_tlvs);
     if (err) {
@@ -2040,17 +2030,10 @@ static int of_dpa_cmd_add_l2_flood(OfDpa *of_dpa, OfDpaGroup *group,
         rocker_tlv_get_le16(group_tlvs[ROCKER_TLV_OF_DPA_GROUP_COUNT]);
 
     tlvs = g_new0(RockerTlv *, group->l2_flood.group_count + 1);
-    if (!tlvs) {
-        return -ROCKER_ENOMEM;
-    }
 
     g_free(group->l2_flood.group_ids);
     group->l2_flood.group_ids =
         g_new0(uint32_t, group->l2_flood.group_count);
-    if (!group->l2_flood.group_ids) {
-        err = -ROCKER_ENOMEM;
-        goto err_out;
-    }
 
     rocker_tlv_parse_nested(tlvs, group->l2_flood.group_count,
                             group_tlvs[ROCKER_TLV_OF_DPA_GROUP_IDS]);
@@ -2157,9 +2140,6 @@ static int of_dpa_cmd_group_add(OfDpa *of_dpa, uint32_t group_id,
     }
 
     group = of_dpa_group_alloc(group_id);
-    if (!group) {
-        return -ROCKER_ENOMEM;
-    }
 
     err = of_dpa_cmd_group_do(of_dpa, group_id, group, group_tlvs);
     if (err) {
diff --git a/hw/net/rocker/rocker_world.c b/hw/net/rocker/rocker_world.c
index 89777e9..f73c534 100644
--- a/hw/net/rocker/rocker_world.c
+++ b/hw/net/rocker/rocker_world.c
@@ -51,13 +51,11 @@ World *world_alloc(Rocker *r, size_t sizeof_private,
 {
     World *w = g_malloc0(sizeof(World) + sizeof_private);
 
-    if (w) {
-        w->r = r;
-        w->type = type;
-        w->ops = ops;
-        if (w->ops->init) {
-            w->ops->init(w);
-        }
+    w->r = r;
+    w->type = type;
+    w->ops = ops;
+    if (w->ops->init) {
+        w->ops->init(w);
     }
 
     return w;
-- 
2.9.3

Re: [Qemu-devel] [PATCH v5 1/4] net/rocker: Remove the dead error handling

[Markus Armbruster]

Mao Zhongyi <maozy.fnst@cn.fujitsu.com> writes:

> Memory allocation functions like world_alloc, desc_ring_alloc etc,
> they are all wrappers around g_malloc, g_new etc. But g_malloc and 
> similar functions doesn't return null. Because they ignore the fact 

don't

> that g_malloc() of 0 bytes returns null. So error checks for these
> allocation failure are superfluous. Now, remove them entirely.
>
> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

There's one more cleanup opportunity:

> diff --git a/hw/net/rocker/rocker_desc.c b/hw/net/rocker/rocker_desc.c
> index ac02797..d0df89a 100644
> --- a/hw/net/rocker/rocker_desc.c
> +++ b/hw/net/rocker/rocker_desc.c
> @@ -65,10 +65,6 @@ char *desc_get_buf(DescInfo *info, bool read_only)
>          info->buf_size = size;
>      }
>  
> -    if (!info->buf) {
> -        return NULL;
> -    }
> -
>      if (pci_dma_read(dev, le64_to_cpu(info->desc.buf_addr), info->buf, size)) {
>          return NULL;
>      }

None of the pci_dma_read() calls outside rocker check the return value.
Just as well, because it always returns 0.  Please clean this up in a
separate followup patch.

[...]

Re: [Qemu-devel] [PATCH v5 1/4] net/rocker: Remove the dead error handling

[Mao Zhongyi]

Hi, Markus


On 05/23/2017 05:27 PM, Markus Armbruster wrote:
> Mao Zhongyi <maozy.fnst@cn.fujitsu.com> writes:
>
>> Memory allocation functions like world_alloc, desc_ring_alloc etc,
>> they are all wrappers around g_malloc, g_new etc. But g_malloc and
>> similar functions doesn't return null. Because they ignore the fact
>
> don't

Will I need to make a separated patch to fix it? or when you merge to
help me repair?

Thanks a lot.

>
>> that g_malloc() of 0 bytes returns null. So error checks for these
>> allocation failure are superfluous. Now, remove them entirely.
>>
>> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Thanks for your quick review:)

>
> There's one more cleanup opportunity:
>
>> diff --git a/hw/net/rocker/rocker_desc.c b/hw/net/rocker/rocker_desc.c
>> index ac02797..d0df89a 100644
>> --- a/hw/net/rocker/rocker_desc.c
>> +++ b/hw/net/rocker/rocker_desc.c
>> @@ -65,10 +65,6 @@ char *desc_get_buf(DescInfo *info, bool read_only)
>>          info->buf_size = size;
>>      }
>>
>> -    if (!info->buf) {
>> -        return NULL;
>> -    }
>> -
>>      if (pci_dma_read(dev, le64_to_cpu(info->desc.buf_addr), info->buf, size)) {
>>          return NULL;
>>      }
>
> None of the pci_dma_read() calls outside rocker check the return value.
> Just as well, because it always returns 0.  Please clean this up in a
> separate followup patch.

Thanks for the reminder. I just read the code, it's true.
Will fix it right away.

Mao

Re: [Qemu-devel] [PATCH v5 1/4] net/rocker: Remove the dead error handling

[Markus Armbruster]

Mao Zhongyi <maozy.fnst@cn.fujitsu.com> writes:

> Hi, Markus
>
>
> On 05/23/2017 05:27 PM, Markus Armbruster wrote:
>> Mao Zhongyi <maozy.fnst@cn.fujitsu.com> writes:
>>
>>> Memory allocation functions like world_alloc, desc_ring_alloc etc,
>>> they are all wrappers around g_malloc, g_new etc. But g_malloc and
>>> similar functions doesn't return null. Because they ignore the fact
>>
>> don't
>
> Will I need to make a separated patch to fix it? or when you merge to
> help me repair?

Perhaps Jason can touch it up on commit.

> Thanks a lot.
>
>>
>>> that g_malloc() of 0 bytes returns null. So error checks for these
>>> allocation failure are superfluous. Now, remove them entirely.
>>>
>>> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
>>
>> Reviewed-by: Markus Armbruster <armbru@redhat.com>
>
> Thanks for your quick review:)

You're welcome.

>> There's one more cleanup opportunity:
>>
>>> diff --git a/hw/net/rocker/rocker_desc.c b/hw/net/rocker/rocker_desc.c
>>> index ac02797..d0df89a 100644
>>> --- a/hw/net/rocker/rocker_desc.c
>>> +++ b/hw/net/rocker/rocker_desc.c
>>> @@ -65,10 +65,6 @@ char *desc_get_buf(DescInfo *info, bool read_only)
>>>          info->buf_size = size;
>>>      }
>>>
>>> -    if (!info->buf) {
>>> -        return NULL;
>>> -    }
>>> -
>>>      if (pci_dma_read(dev, le64_to_cpu(info->desc.buf_addr), info->buf, size)) {
>>>          return NULL;
>>>      }
>>
>> None of the pci_dma_read() calls outside rocker check the return value.
>> Just as well, because it always returns 0.  Please clean this up in a
>> separate followup patch.
>
> Thanks for the reminder. I just read the code, it's true.
> Will fix it right away.

Thanks!

Re: [Qemu-devel] [PATCH v5 1/4] net/rocker: Remove the dead error handling

[Philippe Mathieu-Daudé]

Hi Markus,

On 05/23/2017 06:27 AM, Markus Armbruster wrote:
[...]
> There's one more cleanup opportunity:
>
[...]
>>      if (pci_dma_read(dev, le64_to_cpu(info->desc.buf_addr), info->buf, size)) {
>>          return NULL;
>>      }
>
> None of the pci_dma_read() calls outside rocker check the return value.
> Just as well, because it always returns 0.  Please clean this up in a
> separate followup patch.

It may be the correct way to do it but this sounds like we are missing 
something somewhere... pci_dma_read() calls pci_dma_rw() which always 
returns 0. Why not let it returns void? It is inlined and never used by 
address. Else we should document why returning 0 is correct, and what is 
the reason to not use a void prototype.

pci_dma_rw() calls dma_memory_rw() which does return a boolean value, 
false on success (MEMTX_OK) and true on error (MEMTX_ERROR/DECODE_ERROR)

Regards,

Phil.

Re: [Qemu-devel] [PATCH v5 1/4] net/rocker: Remove the dead error handling

[Markus Armbruster]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Hi Markus,
>
> On 05/23/2017 06:27 AM, Markus Armbruster wrote:
> [...]
>> There's one more cleanup opportunity:
>>
> [...]
>>>      if (pci_dma_read(dev, le64_to_cpu(info->desc.buf_addr), info->buf, size)) {
>>>          return NULL;
>>>      }
>>
>> None of the pci_dma_read() calls outside rocker check the return value.
>> Just as well, because it always returns 0.  Please clean this up in a
>> separate followup patch.
>
> It may be the correct way to do it but this sounds like we are missing
> something somewhere... pci_dma_read() calls pci_dma_rw() which always
> returns 0. Why not let it returns void? It is inlined and never used
> by address. Else we should document why returning 0 is correct, and
> what is the reason to not use a void prototype.
>
> pci_dma_rw() calls dma_memory_rw() which does return a boolean value,
> false on success (MEMTX_OK) and true on error
> (MEMTX_ERROR/DECODE_ERROR)

PCI question.  Michael, Marcel?

Re: [Qemu-devel] [PATCH v5 1/4] net/rocker: Remove the dead error handling

[Marcel Apfelbaum]

----- Original Message -----
> From: "Markus Armbruster" <armbru@redhat.com>
> To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> Cc: qemu-devel@nongnu.org, "Mao Zhongyi" <maozy.fnst@cn.fujitsu.com>, jiri@resnulli.us, jasowang@redhat.com, "Michael
> S. Tsirkin" <mst@redhat.com>, "Marcel Apfelbaum" <marcel@redhat.com>
> Sent: Wednesday, May 24, 2017 8:35:04 AM
> Subject: Re: [Qemu-devel] [PATCH v5 1/4] net/rocker: Remove the dead error handling
> 
> Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> 
> > Hi Markus,
> >
> > On 05/23/2017 06:27 AM, Markus Armbruster wrote:
> > [...]
> >> There's one more cleanup opportunity:
> >>
> > [...]
> >>>      if (pci_dma_read(dev, le64_to_cpu(info->desc.buf_addr), info->buf,
> >>>      size)) {
> >>>          return NULL;
> >>>      }
> >>
> >> None of the pci_dma_read() calls outside rocker check the return value.
> >> Just as well, because it always returns 0.  Please clean this up in a
> >> separate followup patch.
> >
> > It may be the correct way to do it but this sounds like we are missing
> > something somewhere... pci_dma_read() calls pci_dma_rw() which always
> > returns 0. Why not let it returns void? It is inlined and never used
> > by address. Else we should document why returning 0 is correct, and
> > what is the reason to not use a void prototype.
> >
> > pci_dma_rw() calls dma_memory_rw() which does return a boolean value,
> > false on success (MEMTX_OK) and true on error
> > (MEMTX_ERROR/DECODE_ERROR)
> 
> PCI question.  Michael, Marcel?
> 

Hi Markus,

Looking at the git history, pci_dma_rw used to call cpu_physical_memory_rw
which, at that time (commit ec17457), returned void. Since the interface dictated
to return int, 0 is returned as "always OK".

The callers to pci_dma_read did not bother to check it for obvious reasons (even if they should).

In the meantime the memory API has changed to allow returning errors, but since the callers of
pci_dma_rw don't check the return value, why bother to update the PCI DMA?

History aside (and my speculations above), it seems  the right move is to update
the return value and check it by callers, but honestly I don't have any idea
if the emulated devices expect pci dma to fail.
Adding Paolo and David for more insights.

Thanks,
Marcel

Re: [Qemu-devel] [PATCH v5 1/4] net/rocker: Remove the dead error handling

[David Gibson]

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

On Wed, 24 May 2017 08:01:47 -0400 (EDT)
Marcel Apfelbaum <marcel@redhat.com> wrote:

> ----- Original Message -----
> > From: "Markus Armbruster" <armbru@redhat.com>
> > To: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
> > Cc: qemu-devel@nongnu.org, "Mao Zhongyi" <maozy.fnst@cn.fujitsu.com>, jiri@resnulli.us, jasowang@redhat.com, "Michael
> > S. Tsirkin" <mst@redhat.com>, "Marcel Apfelbaum" <marcel@redhat.com>
> > Sent: Wednesday, May 24, 2017 8:35:04 AM
> > Subject: Re: [Qemu-devel] [PATCH v5 1/4] net/rocker: Remove the dead error handling
> > 
> > Philippe Mathieu-Daudé <f4bug@amsat.org> writes:
> >   
> > > Hi Markus,
> > >
> > > On 05/23/2017 06:27 AM, Markus Armbruster wrote:
> > > [...]  
> > >> There's one more cleanup opportunity:
> > >>  
> > > [...]  
> > >>>      if (pci_dma_read(dev, le64_to_cpu(info->desc.buf_addr), info->buf,
> > >>>      size)) {
> > >>>          return NULL;
> > >>>      }  
> > >>
> > >> None of the pci_dma_read() calls outside rocker check the return value.
> > >> Just as well, because it always returns 0.  Please clean this up in a
> > >> separate followup patch.  
> > >
> > > It may be the correct way to do it but this sounds like we are missing
> > > something somewhere... pci_dma_read() calls pci_dma_rw() which always
> > > returns 0. Why not let it returns void? It is inlined and never used
> > > by address. Else we should document why returning 0 is correct, and
> > > what is the reason to not use a void prototype.
> > >
> > > pci_dma_rw() calls dma_memory_rw() which does return a boolean value,
> > > false on success (MEMTX_OK) and true on error
> > > (MEMTX_ERROR/DECODE_ERROR)  
> > 
> > PCI question.  Michael, Marcel?
> >   
> 
> Hi Markus,
> 
> Looking at the git history, pci_dma_rw used to call cpu_physical_memory_rw
> which, at that time (commit ec17457), returned void. Since the interface dictated
> to return int, 0 is returned as "always OK".
> 
> The callers to pci_dma_read did not bother to check it for obvious reasons (even if they should).
> 
> In the meantime the memory API has changed to allow returning errors, but since the callers of
> pci_dma_rw don't check the return value, why bother to update the PCI DMA?
> 
> History aside (and my speculations above), it seems  the right move is to update
> the return value and check it by callers, but honestly I don't have any idea
> if the emulated devices expect pci dma to fail.
> Adding Paolo and David for more insights.

It seems to me that PCI DMA transfers ought to be able to fail, and
devices ought to be able to handle that (to a limited extent).

After all, what will happen if you try to DMA to PCI addresses that
simply aren't mapped.  Or which are in the domain of a vIOMMU which
wither hasn't mapped those addreses, or has them mapped read-only
(meaning host-to-device only in this context).

-- 
David Gibson <dgibson@redhat.com>
Principal Software Engineer, Virtualization, Red Hat

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[Qemu-devel] [PATCH v5 2/4] net/rocker: Plug memory leak in pci_rocker_init()

[Mao Zhongyi]

pci_rocker_init() leaks a World when the name more than 9 chars,
then return a negative value directly, doesn't make a correct
cleanup. So add a new goto label to fix it.

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hw/net/rocker/rocker.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index d01ba9d..6d44f37 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1357,7 +1357,8 @@ static int pci_rocker_init(PCIDevice *dev)
         fprintf(stderr,
                 "rocker: name too long; please shorten to at most %d chars\n",
                 MAX_ROCKER_NAME_LEN);
-        return -EINVAL;
+        err = -EINVAL;
+        goto err_name_too_long;
     }
 
     if (memcmp(&r->fp_start_macaddr, &zero, sizeof(zero)) == 0) {
@@ -1416,6 +1417,7 @@ static int pci_rocker_init(PCIDevice *dev)
 
     return 0;
 
+err_name_too_long:
 err_duplicate:
     rocker_msix_uninit(r);
 err_msix_init:
-- 
2.9.3

Re: [Qemu-devel] [PATCH v5 2/4] net/rocker: Plug memory leak in pci_rocker_init()

[Philippe Mathieu-Daudé]

On 05/23/2017 01:04 AM, Mao Zhongyi wrote:
> pci_rocker_init() leaks a World when the name more than 9 chars,
> then return a negative value directly, doesn't make a correct
> cleanup. So add a new goto label to fix it.
>
> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/net/rocker/rocker.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index d01ba9d..6d44f37 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -1357,7 +1357,8 @@ static int pci_rocker_init(PCIDevice *dev)
>          fprintf(stderr,
>                  "rocker: name too long; please shorten to at most %d chars\n",
>                  MAX_ROCKER_NAME_LEN);
> -        return -EINVAL;
> +        err = -EINVAL;
> +        goto err_name_too_long;
>      }
>
>      if (memcmp(&r->fp_start_macaddr, &zero, sizeof(zero)) == 0) {
> @@ -1416,6 +1417,7 @@ static int pci_rocker_init(PCIDevice *dev)
>
>      return 0;
>
> +err_name_too_long:
>  err_duplicate:
>      rocker_msix_uninit(r);
>  err_msix_init:
>

[Qemu-devel] [PATCH v5 3/4] net/rocker: Convert to realize()

[Mao Zhongyi]

The rocker device still implements the old PCIDeviceClass .init()
instead of the new .realize(). All devices need to be converted to
.realize().

.init() reports errors with fprintf() and return 0 on success, negative
number on failure. Meanwhile, when -device rocker fails, it first report
a specific error, then a generic one, like this:

    $ x86_64-softmmu/qemu-system-x86_64 -device rocker,name=qemu-rocker
    rocker: name too long; please shorten to at most 9 chars
    qemu-system-x86_64: -device rocker,name=qemu-rocker: Device initialization failed

Now, convert it to .realize() that passes errors to its callers via its
errp argument. Also avoid the superfluous second error message. After
the patch, effect like this:

    $ x86_64-softmmu/qemu-system-x86_64 -device rocker,name=qemu-rocker
    qemu-system-x86_64: -device rocker,name=qemu-rocker: name too long; please shorten to at most 9 chars

Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
 hw/net/rocker/rocker.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 6d44f37..2764529 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -1238,20 +1238,18 @@ rollback:
     return err;
 }
 
-static int rocker_msix_init(Rocker *r)
+static int rocker_msix_init(Rocker *r, Error **errp)
 {
     PCIDevice *dev = PCI_DEVICE(r);
     int err;
-    Error *local_err = NULL;
 
     err = msix_init(dev, ROCKER_MSIX_VEC_COUNT(r->fp_ports),
                     &r->msix_bar,
                     ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_TABLE_OFFSET,
                     &r->msix_bar,
                     ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_PBA_OFFSET,
-                    0, &local_err);
+                    0, errp);
     if (err) {
-        error_report_err(local_err);
         return err;
     }
 
@@ -1287,7 +1285,7 @@ static World *rocker_world_type_by_name(Rocker *r, const char *name)
     return NULL;
 }
 
-static int pci_rocker_init(PCIDevice *dev)
+static void pci_rocker_realize(PCIDevice *dev, Error **errp)
 {
     Rocker *r = to_rocker(dev);
     const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
@@ -1305,10 +1303,9 @@ static int pci_rocker_init(PCIDevice *dev)
 
     r->world_dflt = rocker_world_type_by_name(r, r->world_name);
     if (!r->world_dflt) {
-        fprintf(stderr,
-                "rocker: requested world \"%s\" does not exist\n",
+        error_setg(errp,
+                "invalid argument requested world %s does not exist",
                 r->world_name);
-        err = -EINVAL;
         goto err_world_type_by_name;
     }
 
@@ -1328,7 +1325,7 @@ static int pci_rocker_init(PCIDevice *dev)
 
     /* MSI-X init */
 
-    err = rocker_msix_init(r);
+    err = rocker_msix_init(r, errp);
     if (err) {
         goto err_msix_init;
     }
@@ -1340,7 +1337,7 @@ static int pci_rocker_init(PCIDevice *dev)
     }
 
     if (rocker_find(r->name)) {
-        err = -EEXIST;
+        error_setg(errp, "%s already exists", r->name);
         goto err_duplicate;
     }
 
@@ -1354,10 +1351,9 @@ static int pci_rocker_init(PCIDevice *dev)
 #define ROCKER_IFNAMSIZ 16
 #define MAX_ROCKER_NAME_LEN  (ROCKER_IFNAMSIZ - 1 - 3 - 3)
     if (strlen(r->name) > MAX_ROCKER_NAME_LEN) {
-        fprintf(stderr,
-                "rocker: name too long; please shorten to at most %d chars\n",
+        error_setg(errp,
+                "name too long; please shorten to at most %d chars",
                 MAX_ROCKER_NAME_LEN);
-        err = -EINVAL;
         goto err_name_too_long;
     }
 
@@ -1415,7 +1411,7 @@ static int pci_rocker_init(PCIDevice *dev)
 
     QLIST_INSERT_HEAD(&rockers, r, next);
 
-    return 0;
+    return;
 
 err_name_too_long:
 err_duplicate:
@@ -1429,7 +1425,6 @@ err_world_type_by_name:
             world_free(r->worlds[i]);
         }
     }
-    return err;
 }
 
 static void pci_rocker_uninit(PCIDevice *dev)
@@ -1514,7 +1509,7 @@ static void rocker_class_init(ObjectClass *klass, void *data)
     DeviceClass *dc = DEVICE_CLASS(klass);
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
 
-    k->init = pci_rocker_init;
+    k->realize = pci_rocker_realize;
     k->exit = pci_rocker_uninit;
     k->vendor_id = PCI_VENDOR_ID_REDHAT;
     k->device_id = PCI_DEVICE_ID_REDHAT_ROCKER;
-- 
2.9.3

Re: [Qemu-devel] [PATCH v5 3/4] net/rocker: Convert to realize()

[Philippe Mathieu-Daudé]

On 05/23/2017 01:04 AM, Mao Zhongyi wrote:
> The rocker device still implements the old PCIDeviceClass .init()
> instead of the new .realize(). All devices need to be converted to
> .realize().
>
> .init() reports errors with fprintf() and return 0 on success, negative
> number on failure. Meanwhile, when -device rocker fails, it first report
> a specific error, then a generic one, like this:
>
>     $ x86_64-softmmu/qemu-system-x86_64 -device rocker,name=qemu-rocker
>     rocker: name too long; please shorten to at most 9 chars
>     qemu-system-x86_64: -device rocker,name=qemu-rocker: Device initialization failed
>
> Now, convert it to .realize() that passes errors to its callers via its
> errp argument. Also avoid the superfluous second error message. After
> the patch, effect like this:
>
>     $ x86_64-softmmu/qemu-system-x86_64 -device rocker,name=qemu-rocker
>     qemu-system-x86_64: -device rocker,name=qemu-rocker: name too long; please shorten to at most 9 chars
>
> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/net/rocker/rocker.c | 27 +++++++++++----------------
>  1 file changed, 11 insertions(+), 16 deletions(-)
>
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index 6d44f37..2764529 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -1238,20 +1238,18 @@ rollback:
>      return err;
>  }
>
> -static int rocker_msix_init(Rocker *r)
> +static int rocker_msix_init(Rocker *r, Error **errp)
>  {
>      PCIDevice *dev = PCI_DEVICE(r);
>      int err;
> -    Error *local_err = NULL;
>
>      err = msix_init(dev, ROCKER_MSIX_VEC_COUNT(r->fp_ports),
>                      &r->msix_bar,
>                      ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_TABLE_OFFSET,
>                      &r->msix_bar,
>                      ROCKER_PCI_MSIX_BAR_IDX, ROCKER_PCI_MSIX_PBA_OFFSET,
> -                    0, &local_err);
> +                    0, errp);
>      if (err) {
> -        error_report_err(local_err);
>          return err;
>      }
>
> @@ -1287,7 +1285,7 @@ static World *rocker_world_type_by_name(Rocker *r, const char *name)
>      return NULL;
>  }
>
> -static int pci_rocker_init(PCIDevice *dev)
> +static void pci_rocker_realize(PCIDevice *dev, Error **errp)
>  {
>      Rocker *r = to_rocker(dev);
>      const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
> @@ -1305,10 +1303,9 @@ static int pci_rocker_init(PCIDevice *dev)
>
>      r->world_dflt = rocker_world_type_by_name(r, r->world_name);
>      if (!r->world_dflt) {
> -        fprintf(stderr,
> -                "rocker: requested world \"%s\" does not exist\n",
> +        error_setg(errp,
> +                "invalid argument requested world %s does not exist",
>                  r->world_name);
> -        err = -EINVAL;
>          goto err_world_type_by_name;
>      }
>
> @@ -1328,7 +1325,7 @@ static int pci_rocker_init(PCIDevice *dev)
>
>      /* MSI-X init */
>
> -    err = rocker_msix_init(r);
> +    err = rocker_msix_init(r, errp);
>      if (err) {
>          goto err_msix_init;
>      }
> @@ -1340,7 +1337,7 @@ static int pci_rocker_init(PCIDevice *dev)
>      }
>
>      if (rocker_find(r->name)) {
> -        err = -EEXIST;
> +        error_setg(errp, "%s already exists", r->name);
>          goto err_duplicate;
>      }
>
> @@ -1354,10 +1351,9 @@ static int pci_rocker_init(PCIDevice *dev)
>  #define ROCKER_IFNAMSIZ 16
>  #define MAX_ROCKER_NAME_LEN  (ROCKER_IFNAMSIZ - 1 - 3 - 3)
>      if (strlen(r->name) > MAX_ROCKER_NAME_LEN) {
> -        fprintf(stderr,
> -                "rocker: name too long; please shorten to at most %d chars\n",
> +        error_setg(errp,
> +                "name too long; please shorten to at most %d chars",
>                  MAX_ROCKER_NAME_LEN);
> -        err = -EINVAL;
>          goto err_name_too_long;
>      }
>
> @@ -1415,7 +1411,7 @@ static int pci_rocker_init(PCIDevice *dev)
>
>      QLIST_INSERT_HEAD(&rockers, r, next);
>
> -    return 0;
> +    return;
>
>  err_name_too_long:
>  err_duplicate:
> @@ -1429,7 +1425,6 @@ err_world_type_by_name:
>              world_free(r->worlds[i]);
>          }
>      }
> -    return err;
>  }
>
>  static void pci_rocker_uninit(PCIDevice *dev)
> @@ -1514,7 +1509,7 @@ static void rocker_class_init(ObjectClass *klass, void *data)
>      DeviceClass *dc = DEVICE_CLASS(klass);
>      PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>
> -    k->init = pci_rocker_init;
> +    k->realize = pci_rocker_realize;
>      k->exit = pci_rocker_uninit;
>      k->vendor_id = PCI_VENDOR_ID_REDHAT;
>      k->device_id = PCI_DEVICE_ID_REDHAT_ROCKER;
>

[Qemu-devel] [PATCH v5 4/4] net/rocker: Fix the unusual macro name

[Mao Zhongyi]

Suggested-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
---
 hw/net/rocker/rocker.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
index 2764529..f8a32f7 100644
--- a/hw/net/rocker/rocker.c
+++ b/hw/net/rocker/rocker.c
@@ -69,10 +69,10 @@ struct rocker {
     QLIST_ENTRY(rocker) next;
 };
 
-#define ROCKER "rocker"
+#define TYPE_ROCKER "rocker"
 
-#define to_rocker(obj) \
-    OBJECT_CHECK(Rocker, (obj), ROCKER)
+#define ROCKER(obj) \
+    OBJECT_CHECK(Rocker, (obj), TYPE_ROCKER)
 
 static QLIST_HEAD(, rocker) rockers;
 
@@ -1287,7 +1287,7 @@ static World *rocker_world_type_by_name(Rocker *r, const char *name)
 
 static void pci_rocker_realize(PCIDevice *dev, Error **errp)
 {
-    Rocker *r = to_rocker(dev);
+    Rocker *r = ROCKER(dev);
     const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
     const MACAddr dflt = { .a = { 0x52, 0x54, 0x00, 0x12, 0x35, 0x01 } };
     static int sw_index;
@@ -1333,7 +1333,7 @@ static void pci_rocker_realize(PCIDevice *dev, Error **errp)
     /* validate switch properties */
 
     if (!r->name) {
-        r->name = g_strdup(ROCKER);
+        r->name = g_strdup(TYPE_ROCKER);
     }
 
     if (rocker_find(r->name)) {
@@ -1429,7 +1429,7 @@ err_world_type_by_name:
 
 static void pci_rocker_uninit(PCIDevice *dev)
 {
-    Rocker *r = to_rocker(dev);
+    Rocker *r = ROCKER(dev);
     int i;
 
     QLIST_REMOVE(r, next);
@@ -1462,7 +1462,7 @@ static void pci_rocker_uninit(PCIDevice *dev)
 
 static void rocker_reset(DeviceState *dev)
 {
-    Rocker *r = to_rocker(dev);
+    Rocker *r = ROCKER(dev);
     int i;
 
     for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
@@ -1500,7 +1500,7 @@ static Property rocker_properties[] = {
 };
 
 static const VMStateDescription rocker_vmsd = {
-    .name = ROCKER,
+    .name = TYPE_ROCKER,
     .unmigratable = 1,
 };
 
@@ -1523,7 +1523,7 @@ static void rocker_class_init(ObjectClass *klass, void *data)
 }
 
 static const TypeInfo rocker_info = {
-    .name          = ROCKER,
+    .name          = TYPE_ROCKER,
     .parent        = TYPE_PCI_DEVICE,
     .instance_size = sizeof(Rocker),
     .class_init    = rocker_class_init,
-- 
2.9.3

Re: [Qemu-devel] [PATCH v5 4/4] net/rocker: Fix the unusual macro name

[Markus Armbruster]

Mao Zhongyi <maozy.fnst@cn.fujitsu.com> writes:

> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v5 4/4] net/rocker: Fix the unusual macro name

[Philippe Mathieu-Daudé]

On 05/23/2017 01:04 AM, Mao Zhongyi wrote:
> Suggested-by: Markus Armbruster <armbru@redhat.com>
> Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  hw/net/rocker/rocker.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/hw/net/rocker/rocker.c b/hw/net/rocker/rocker.c
> index 2764529..f8a32f7 100644
> --- a/hw/net/rocker/rocker.c
> +++ b/hw/net/rocker/rocker.c
> @@ -69,10 +69,10 @@ struct rocker {
>      QLIST_ENTRY(rocker) next;
>  };
>
> -#define ROCKER "rocker"
> +#define TYPE_ROCKER "rocker"
>
> -#define to_rocker(obj) \
> -    OBJECT_CHECK(Rocker, (obj), ROCKER)
> +#define ROCKER(obj) \
> +    OBJECT_CHECK(Rocker, (obj), TYPE_ROCKER)
>
>  static QLIST_HEAD(, rocker) rockers;
>
> @@ -1287,7 +1287,7 @@ static World *rocker_world_type_by_name(Rocker *r, const char *name)
>
>  static void pci_rocker_realize(PCIDevice *dev, Error **errp)
>  {
> -    Rocker *r = to_rocker(dev);
> +    Rocker *r = ROCKER(dev);
>      const MACAddr zero = { .a = { 0, 0, 0, 0, 0, 0 } };
>      const MACAddr dflt = { .a = { 0x52, 0x54, 0x00, 0x12, 0x35, 0x01 } };
>      static int sw_index;
> @@ -1333,7 +1333,7 @@ static void pci_rocker_realize(PCIDevice *dev, Error **errp)
>      /* validate switch properties */
>
>      if (!r->name) {
> -        r->name = g_strdup(ROCKER);
> +        r->name = g_strdup(TYPE_ROCKER);
>      }
>
>      if (rocker_find(r->name)) {
> @@ -1429,7 +1429,7 @@ err_world_type_by_name:
>
>  static void pci_rocker_uninit(PCIDevice *dev)
>  {
> -    Rocker *r = to_rocker(dev);
> +    Rocker *r = ROCKER(dev);
>      int i;
>
>      QLIST_REMOVE(r, next);
> @@ -1462,7 +1462,7 @@ static void pci_rocker_uninit(PCIDevice *dev)
>
>  static void rocker_reset(DeviceState *dev)
>  {
> -    Rocker *r = to_rocker(dev);
> +    Rocker *r = ROCKER(dev);
>      int i;
>
>      for (i = 0; i < ROCKER_WORLD_TYPE_MAX; i++) {
> @@ -1500,7 +1500,7 @@ static Property rocker_properties[] = {
>  };
>
>  static const VMStateDescription rocker_vmsd = {
> -    .name = ROCKER,
> +    .name = TYPE_ROCKER,
>      .unmigratable = 1,
>  };
>
> @@ -1523,7 +1523,7 @@ static void rocker_class_init(ObjectClass *klass, void *data)
>  }
>
>  static const TypeInfo rocker_info = {
> -    .name          = ROCKER,
> +    .name          = TYPE_ROCKER,
>      .parent        = TYPE_PCI_DEVICE,
>      .instance_size = sizeof(Rocker),
>      .class_init    = rocker_class_init,
>

Re: [Qemu-devel] [PATCH v5 0/4] Convert to realize and fix error handling

[Mao Zhongyi]

Hi, Jason

This was posted over 2 months ago with two R-Bs(Markus & Philippe),
did it get merged or dropped?

Thanks,
Mao

On 05/23/2017 12:04 PM, Mao Zhongyi wrote:
> v5:
> * Patch 1 removed the dead error handling that was previously missing.
> * Patch 2 and 3 has not changed.
> * Patch 4 is a new patch to fix the unusual macro name.
>
> v4:
> * Patch 1 is following Markus's suggestion that remove the dead error
>   handling.
> * Patch 2 is separate from patch 1 to plug the memory leak in the v3.
> * Patch 3 is based on the patch 1 in the v3. Meanwhile, dorp the superfluous
>   prefix "rocker:" and adjust the commit message.
>
> v3:
> * Following Jason's suggstion that add suitable error message to
>   each error site.
> * Modified the commit message to make it easier to read.
>
> v2:
> * Following Philippe's suggestion that shorten the patch subject
>   "hw/net/rocker/rocker" to "net/rocker".
> * Use a consistent log format to report error message.
> * Add a specific goto label "err_name_too_long" to make a correct
>   cleanup.
>
> Mao Zhongyi (4):
>   net/rocker: Remove the dead error handling
>   net/rocker: Plug memory leak in pci_rocker_init()
>   net/rocker: Convert to realize()
>   net/rocker: Fix the unusual macro name
>
>  hw/net/rocker/rocker.c        | 93 ++++++++++---------------------------------
>  hw/net/rocker/rocker_desc.c   | 10 -----
>  hw/net/rocker/rocker_fp.c     |  4 --
>  hw/net/rocker/rocker_of_dpa.c | 20 ----------
>  hw/net/rocker/rocker_world.c  | 12 +++---
>  5 files changed, 27 insertions(+), 112 deletions(-)
>

Re: [Qemu-devel] [PATCH v5 0/4] Convert to realize and fix error handling

[Jason Wang]

On 2017年07月26日 11:45, Mao Zhongyi wrote:
> Hi, Jason
>
> This was posted over 2 months ago with two R-Bs(Markus & Philippe),
> did it get merged or dropped?
>
> Thanks,
> Mao

Hi:

Want to merge but it does not apply cleanly on HEAD. Could you please 
rebase and post a V6?

Thanks

Re: [Qemu-devel] [PATCH v5 0/4] Convert to realize and fix error handling

[Mao Zhongyi]

On 07/26/2017 12:09 PM, Jason Wang wrote:
>
>
> On 2017年07月26日 11:45, Mao Zhongyi wrote:
>> Hi, Jason
>>
>> This was posted over 2 months ago with two R-Bs(Markus & Philippe),
>> did it get merged or dropped?
>>
>> Thanks,
>> Mao
>
> Hi:
>
> Want to merge but it does not apply cleanly on HEAD. Could you please rebase and post a V6?
>

OK, I will

Thanks :)


> Thanks
>
>
>
>