[Qemu-devel] [PATCH v2 0/4] fixes for pci tree, v2

[Qemu-devel] [PATCH v2 0/4] fixes for pci tree, v2

[Hu Tao]

Michael,

This is fixes for your pci tree v2. please review. thanks!

changes to v1:

  1. adds the missing query-memdev and change into querying qom for memdevs(patch 1)
  2. tweak error message (patch 2)
  3. improve error handling for memory-backend-file (patch 3)
  4. fixup the problem that nodes not start from 0 (patch 4)

Hu Tao (4):
  qmp: add query-memdev
  check if we have space left for hotplugged memory
  memory-backend-file: improve error handling
  fixup! numa: add -numa node,memdev= option

 exec.c           | 26 ++++++++++++-----
 hw/mem/pc-dimm.c |  7 +++++
 numa.c           | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 qapi-schema.json | 40 +++++++++++++++++++++++++
 qmp-commands.hx  | 38 ++++++++++++++++++++++++
 5 files changed, 191 insertions(+), 9 deletions(-)

-- 
1.9.3

[Qemu-devel] [PATCH v2 1/4] qmp: add query-memdev

[Hu Tao]

Add qmp command query-memdev to query for information
of memory devices

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 numa.c           | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json | 40 +++++++++++++++++++++++++++
 qmp-commands.hx  | 38 +++++++++++++++++++++++++
 3 files changed, 162 insertions(+)

diff --git a/numa.c b/numa.c
index 8ba5a38..bcbafd2 100644
--- a/numa.c
+++ b/numa.c
@@ -34,6 +34,7 @@
 #include "qapi/qmp/qerror.h"
 #include "hw/boards.h"
 #include "sysemu/hostmem.h"
+#include "qmp-commands.h"
 
 QemuOptsList qemu_numa_opts = {
     .name = "numa",
@@ -280,3 +281,86 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
         addr += size;
     }
 }
+
+static int query_memdev(Object *obj, void *opaque)
+{
+    MemdevList **list = opaque;
+    Error *err = NULL;
+
+    if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
+        MemdevList *m = g_malloc0(sizeof(*m));
+
+        m->value = g_malloc0(sizeof(*m->value));
+
+        m->value->size = object_property_get_int(obj, "size",
+                                                 &err);
+        if (err) {
+            goto error;
+        }
+
+        m->value->merge = object_property_get_bool(obj, "merge",
+                                                   &err);
+        if (err) {
+            goto error;
+        }
+
+        m->value->dump = object_property_get_bool(obj, "dump",
+                                                  &err);
+        if (err) {
+            goto error;
+        }
+
+        m->value->prealloc = object_property_get_bool(obj,
+                                                      "prealloc", &err);
+        if (err) {
+            goto error;
+        }
+
+        m->value->policy = object_property_get_enum(obj,
+                                                    "policy",
+                                                    HostMemPolicy_lookup,
+                                                    &err);
+        if (err) {
+            goto error;
+        }
+
+        object_property_get_uint16List(obj, "host-nodes",
+                                       &m->value->host_nodes, &err);
+        if (err) {
+            goto error;
+        }
+
+        m->next = *list;
+        *list = m;
+    }
+
+    return 0;
+error:
+    return -1;
+}
+
+MemdevList *qmp_query_memdev(Error **errp)
+{
+    Object *obj;
+    MemdevList *list = NULL, *m;
+
+    obj = object_resolve_path("/objects", NULL);
+    if (obj == NULL) {
+        return NULL;
+    }
+
+    if (object_child_foreach(obj, query_memdev, &list) != 0) {
+        goto error;
+    }
+
+    return list;
+
+error:
+    while (list) {
+        m = list;
+        list = list->next;
+        g_free(m->value);
+        g_free(m);
+    }
+    return NULL;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index fa1b876..4c367d0 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4796,3 +4796,43 @@
 ##
 { 'enum': 'HostMemPolicy',
   'data': [ 'default', 'preferred', 'bind', 'interleave' ] }
+
+##
+# @Memdev:
+#
+# Information of memory device
+#
+# @size: memory device size
+#
+# @merge: enables or disables memory merge support
+#
+# @dump: includes memory device's memory in a core dump or not
+#
+# @prealloc: enables or disables memory preallocation
+#
+# @host-nodes: host nodes for its memory policy
+#
+# @policy: memory policy of memory device
+#
+# Since: 2.1
+##
+
+{ 'type': 'Memdev',
+  'data': {
+    'size':       'size',
+    'merge':      'bool',
+    'dump':       'bool',
+    'prealloc':   'bool',
+    'host-nodes': ['uint16'],
+    'policy':     'HostMemPolicy' }}
+
+##
+# @query-memdev:
+#
+# Returns information for all memory devices.
+#
+# Returns: a list of @Memdev.
+#
+# Since: 2.1
+##
+{ 'command': 'query-memdev', 'returns': ['Memdev'] }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index d8aa4ed..bb34cd8 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3572,3 +3572,41 @@ Example:
                    } } ] }
 
 EQMP
+
+    {
+        .name       = "query-memdev",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_memdev,
+    },
+
+SQMP
+query-memdev
+------------
+
+Show memory devices information.
+
+
+Example (1):
+
+-> { "execute": "query-memdev" }
+<- { "return": [
+       {
+         "size": 536870912,
+         "merge": false,
+         "dump": true,
+         "prealloc": false,
+         "host-nodes": [0, 1],
+         "policy": "bind"
+       },
+       {
+         "size": 536870912,
+         "merge": false,
+         "dump": true,
+         "prealloc": true,
+         "host-nodes": [2, 3],
+         "policy": "preferred"
+       }
+     ]
+   }
+
+EQMP
-- 
1.9.3

[Qemu-devel] [PATCH v2 2/4] check if we have space left for hotplugged memory

[Hu Tao]

If pc-dimm is specified on qemu command line, but only with
-m size (aka not -m size,maxmem,slots) then qemu will core dump.

This patch fixes the problem.

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 hw/mem/pc-dimm.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 8c26568..a57b9a3 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -107,6 +107,13 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
     uint64_t new_addr, ret = 0;
     uint64_t address_space_end = address_space_start + address_space_size;
 
+    if (address_space_size == 0) {
+        error_setg(errp, "no space left for hotplugged memory. did you forget "
+                   "maxmem and slots on "
+                   "-m(aka -m size,maxmem=maxmem,slots=slots)?");
+        goto out;
+    }
+
     assert(address_space_end > address_space_size);
     object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list);
 
-- 
1.9.3

[Qemu-devel] [PATCH v2 3/4] memory-backend-file: improve error handling

[Hu Tao]

This patch fixes two problems of memory-backend-file:

1. If user adds a memory-backend-file object using object_add command,
   specifying a non-existing directory for property mem-path, qemu
   will core dump with message:

     /nonexistingdir: No such file or directory
     Bad ram offset fffffffffffff000
     Aborted (core dumped)

2. If user adds a memory-backend-file object using object_add command,
   specifying a size that is less than huge page size, qemu
   will core dump with message:

     Bad ram offset fffffffffffff000
     Aborted (core dumped)

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 exec.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/exec.c b/exec.c
index 8705cc5..a6afb4d 100644
--- a/exec.c
+++ b/exec.c
@@ -997,7 +997,7 @@ void qemu_mutex_unlock_ramlist(void)
 
 #define HUGETLBFS_MAGIC       0x958458f6
 
-static long gethugepagesize(const char *path)
+static long gethugepagesize(const char *path, Error **errp)
 {
     struct statfs fs;
     int ret;
@@ -1007,7 +1007,7 @@ static long gethugepagesize(const char *path)
     } while (ret != 0 && errno == EINTR);
 
     if (ret != 0) {
-        perror(path);
+        error_setg_errno(errp, errno, "failed to stat file %s", path);
         return 0;
     }
 
@@ -1025,17 +1025,19 @@ static void *file_ram_alloc(RAMBlock *block,
     char *filename;
     char *sanitized_name;
     char *c;
-    void *area;
+    void *area = NULL;
     int fd;
     unsigned long hpagesize;
 
-    hpagesize = gethugepagesize(path);
+    hpagesize = gethugepagesize(path, errp);
     if (!hpagesize) {
         goto error;
     }
 
     if (memory < hpagesize) {
-        return NULL;
+        error_setg(errp, "memory size " RAM_ADDR_FMT " should be larger "
+                   "than huge page size %" PRIx64, memory, hpagesize);
+        goto error;
     }
 
     if (kvm_enabled() && !kvm_has_sync_mmu()) {
@@ -1095,8 +1097,8 @@ static void *file_ram_alloc(RAMBlock *block,
     return area;
 
 error:
-    if (mem_prealloc) {
-        exit(1);
+    if (area && area != MAP_FAILED) {
+        munmap(area, memory);
     }
     return NULL;
 }
@@ -1279,6 +1281,7 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
                                     Error **errp)
 {
     RAMBlock *new_block;
+    ram_addr_t addr;
 
     if (xen_enabled()) {
         error_setg(errp, "-mem-path not supported with Xen\n");
@@ -1308,7 +1311,14 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
         return -1;
     }
 
-    return ram_block_add(new_block);
+    addr = ram_block_add(new_block);
+    if (addr == -1) {
+        g_free(new_block);
+        error_setg(errp, "failed to allocate memory\n");
+        return -1;
+    }
+
+    return addr;
 }
 #endif
 
-- 
1.9.3

[Qemu-devel] [PATCH v2 4/4] fixup! numa: add -numa node, memdev= option

[Hu Tao]

note: this is a fixup that can be squashed into commit 0886aa7db0e4.

if numa node number doesn't start from 0 then qemu will core dump.

cmd line to reproduce:

./x86_64-softmmu/qemu-system-x86_64 -hda
/home/data/libvirt-images/f18.img  -m 128M,maxmem=2G,slots=3 -qmp
unix:/tmp/m,server,nowait -monitor stdio -enable-kvm -object
memory-backend-ram,id=m1,size=128M -numa node,nodeid=1,cpus=1,memdev=m1

Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
---
 numa.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/numa.c b/numa.c
index bcbafd2..eef0717 100644
--- a/numa.c
+++ b/numa.c
@@ -266,10 +266,13 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
     }
 
     memory_region_init(mr, owner, name, ram_size);
-    for (i = 0; i < nb_numa_nodes; i++) {
+    for (i = 0; i < MAX_NODES; i++) {
         Error *local_err = NULL;
         uint64_t size = numa_info[i].node_mem;
         HostMemoryBackend *backend = numa_info[i].node_memdev;
+        if (!backend) {
+            continue;
+        }
         MemoryRegion *seg = host_memory_backend_get_memory(backend, &local_err);
         if (local_err) {
             qerror_report_err(local_err);
-- 
1.9.3

Re: [Qemu-devel] [PATCH v2 2/4] check if we have space left for hotplugged memory

[Igor Mammedov]

On Mon, 16 Jun 2014 18:05:42 +0800
Hu Tao <hutao@cn.fujitsu.com> wrote:

> If pc-dimm is specified on qemu command line, but only with
> -m size (aka not -m size,maxmem,slots) then qemu will core dump.
> 
> This patch fixes the problem.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/mem/pc-dimm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 8c26568..a57b9a3 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -107,6 +107,13 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
>      uint64_t new_addr, ret = 0;
>      uint64_t address_space_end = address_space_start + address_space_size;
>  
> +    if (address_space_size == 0) {
> +        error_setg(errp, "no space left for hotplugged memory. did you forget "
> +                   "maxmem and slots on "
> +                   "-m(aka -m size,maxmem=maxmem,slots=slots)?");
CLI format here could get out off sync from actual one. Maybe just
"no space left in hotplug memory range" 

> +        goto out;
> +    }
> +
>      assert(address_space_end > address_space_size);
>      object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list);
>  

Re: [Qemu-devel] [PATCH v2 2/4] check if we have space left for hotplugged memory

[Igor Mammedov]

On Mon, 16 Jun 2014 18:05:42 +0800
Hu Tao <hutao@cn.fujitsu.com> wrote:

> If pc-dimm is specified on qemu command line, but only with
> -m size (aka not -m size,maxmem,slots) then qemu will core dump.
> 
> This patch fixes the problem.
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  hw/mem/pc-dimm.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
> index 8c26568..a57b9a3 100644
> --- a/hw/mem/pc-dimm.c
> +++ b/hw/mem/pc-dimm.c
> @@ -107,6 +107,13 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
another suggestion is to put check in pc_dimm_plug() hotplug handler.

>      uint64_t new_addr, ret = 0;
>      uint64_t address_space_end = address_space_start + address_space_size;
>  
> +    if (address_space_size == 0) {
> +        error_setg(errp, "no space left for hotplugged memory. did you forget "
> +                   "maxmem and slots on "
> +                   "-m(aka -m size,maxmem=maxmem,slots=slots)?");
> +        goto out;
> +    }
> +
>      assert(address_space_end > address_space_size);
>      object_child_foreach(qdev_get_machine(), pc_dimm_built_list, &list);
>  

Re: [Qemu-devel] [PATCH v2 0/4] fixes for pci tree, v2

[Michael S. Tsirkin]

On Mon, Jun 16, 2014 at 06:05:40PM +0800, Hu Tao wrote:
> Michael,
> 
> This is fixes for your pci tree v2. please review. thanks!

Patches 1,4 applied. Thanks!

> changes to v1:
> 
>   1. adds the missing query-memdev and change into querying qom for memdevs(patch 1)
>   2. tweak error message (patch 2)
>   3. improve error handling for memory-backend-file (patch 3)
>   4. fixup the problem that nodes not start from 0 (patch 4)
> 
> Hu Tao (4):
>   qmp: add query-memdev
>   check if we have space left for hotplugged memory
>   memory-backend-file: improve error handling
>   fixup! numa: add -numa node,memdev= option
> 
>  exec.c           | 26 ++++++++++++-----
>  hw/mem/pc-dimm.c |  7 +++++
>  numa.c           | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  qapi-schema.json | 40 +++++++++++++++++++++++++
>  qmp-commands.hx  | 38 ++++++++++++++++++++++++
>  5 files changed, 191 insertions(+), 9 deletions(-)
> 
> -- 
> 1.9.3

Re: [Qemu-devel] [PATCH v2 1/4] qmp: add query-memdev

[Eric Blake]

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

On 06/16/2014 04:05 AM, Hu Tao wrote:
> Add qmp command query-memdev to query for information
> of memory devices
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  numa.c           | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi-schema.json | 40 +++++++++++++++++++++++++++
>  qmp-commands.hx  | 38 +++++++++++++++++++++++++
>  3 files changed, 162 insertions(+)
> 
> diff --git a/numa.c b/numa.c
> index 8ba5a38..bcbafd2 100644
> --- a/numa.c
> +++ b/numa.c
> @@ -34,6 +34,7 @@
>  #include "qapi/qmp/qerror.h"
>  #include "hw/boards.h"
>  #include "sysemu/hostmem.h"
> +#include "qmp-commands.h"
>  
>  QemuOptsList qemu_numa_opts = {
>      .name = "numa",
> @@ -280,3 +281,86 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
>          addr += size;
>      }
>  }
> +
> +static int query_memdev(Object *obj, void *opaque)
> +{
> +    MemdevList **list = opaque;
> +    Error *err = NULL;

This variable is usually named local_err.

> +
> +    if (object_dynamic_cast(obj, TYPE_MEMORY_BACKEND)) {
> +        MemdevList *m = g_malloc0(sizeof(*m));
> +
> +        m->value = g_malloc0(sizeof(*m->value));
> +
> +        m->value->size = object_property_get_int(obj, "size",
> +                                                 &err);
> +        if (err) {
> +            goto error;
> +        }

Is this error ever likely, or would it represent a programming error, in
which case passing &error_abort would avoid the need to check for
failure afterwords.  Similarly for all the other properties.


> +query-memdev
> +------------
> +
> +Show memory devices information.

Makes sense, but took me a couple reads.  Might be better as:

Show information on memory devices.

> +
> +
> +Example (1):

What is the "(1)" for?  I'd just drop it (probably copy-and-paste from
another command that actually had more than one example)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


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

Re: [Qemu-devel] [PATCH v2 0/4] fixes for pci tree, v2

[Hu Tao]

On Mon, Jun 16, 2014 at 02:09:33PM +0300, Michael S. Tsirkin wrote:
> On Mon, Jun 16, 2014 at 06:05:40PM +0800, Hu Tao wrote:
> > Michael,
> > 
> > This is fixes for your pci tree v2. please review. thanks!
> 
> Patches 1,4 applied. Thanks!

What about patches 2,3? If you need I can resend patch 2 based on Igor's
comments.   About patch 3, I assume you leave it as it is not a
regression.

> 
> > changes to v1:
> > 
> >   1. adds the missing query-memdev and change into querying qom for memdevs(patch 1)
> >   2. tweak error message (patch 2)
> >   3. improve error handling for memory-backend-file (patch 3)
> >   4. fixup the problem that nodes not start from 0 (patch 4)
> > 
> > Hu Tao (4):
> >   qmp: add query-memdev
> >   check if we have space left for hotplugged memory
> >   memory-backend-file: improve error handling
> >   fixup! numa: add -numa node,memdev= option
> > 
> >  exec.c           | 26 ++++++++++++-----
> >  hw/mem/pc-dimm.c |  7 +++++
> >  numa.c           | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> >  qapi-schema.json | 40 +++++++++++++++++++++++++
> >  qmp-commands.hx  | 38 ++++++++++++++++++++++++
> >  5 files changed, 191 insertions(+), 9 deletions(-)
> > 
> > -- 
> > 1.9.3

Re: [Qemu-devel] [PATCH v2 3/4] memory-backend-file: improve error handling

[Igor Mammedov]

On Mon, 16 Jun 2014 18:05:43 +0800
Hu Tao <hutao@cn.fujitsu.com> wrote:

> This patch fixes two problems of memory-backend-file:
> 
> 1. If user adds a memory-backend-file object using object_add command,
>    specifying a non-existing directory for property mem-path, qemu
>    will core dump with message:
> 
>      /nonexistingdir: No such file or directory
>      Bad ram offset fffffffffffff000
>      Aborted (core dumped)
> 
> 2. If user adds a memory-backend-file object using object_add command,
>    specifying a size that is less than huge page size, qemu
>    will core dump with message:
> 
>      Bad ram offset fffffffffffff000
>      Aborted (core dumped)
> 
> Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> ---
>  exec.c | 26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/exec.c b/exec.c
> index 8705cc5..a6afb4d 100644
> --- a/exec.c
> +++ b/exec.c
[...]
> @@ -1308,7 +1311,14 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
>          return -1;
>      }
>  
> -    return ram_block_add(new_block);
> +    addr = ram_block_add(new_block);
> +    if (addr == -1) {
how this hunk is relevant to commit message?

and more important in what case ram_block_add() returns -1?

> +        g_free(new_block);
> +        error_setg(errp, "failed to allocate memory\n");
> +        return -1;
> +    }
> +
> +    return addr;
>  }
>  #endif
>  

Re: [Qemu-devel] [PATCH v2 3/4] memory-backend-file: improve error handling

[Hu Tao]

On Tue, Jun 17, 2014 at 12:03:13PM +0200, Igor Mammedov wrote:
> On Mon, 16 Jun 2014 18:05:43 +0800
> Hu Tao <hutao@cn.fujitsu.com> wrote:
> 
> > This patch fixes two problems of memory-backend-file:
> > 
> > 1. If user adds a memory-backend-file object using object_add command,
> >    specifying a non-existing directory for property mem-path, qemu
> >    will core dump with message:
> > 
> >      /nonexistingdir: No such file or directory
> >      Bad ram offset fffffffffffff000
> >      Aborted (core dumped)
> > 
> > 2. If user adds a memory-backend-file object using object_add command,
> >    specifying a size that is less than huge page size, qemu
> >    will core dump with message:
> > 
> >      Bad ram offset fffffffffffff000
> >      Aborted (core dumped)
> > 
> > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > ---
> >  exec.c | 26 ++++++++++++++++++--------
> >  1 file changed, 18 insertions(+), 8 deletions(-)
> > 
> > diff --git a/exec.c b/exec.c
> > index 8705cc5..a6afb4d 100644
> > --- a/exec.c
> > +++ b/exec.c
> [...]
> > @@ -1308,7 +1311,14 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> >          return -1;
> >      }
> >  
> > -    return ram_block_add(new_block);
> > +    addr = ram_block_add(new_block);
> > +    if (addr == -1) {
> how this hunk is relevant to commit message?
> 
> and more important in what case ram_block_add() returns -1?

See patch 03 int v1(titled [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory)
It is not included in this version but we'll fix it after the merge.

> 
> > +        g_free(new_block);
> > +        error_setg(errp, "failed to allocate memory\n");
> > +        return -1;
> > +    }
> > +
> > +    return addr;
> >  }
> >  #endif
> >  

Re: [Qemu-devel] [PATCH v2 0/4] fixes for pci tree, v2

[Michael S. Tsirkin]

On Tue, Jun 17, 2014 at 04:47:53PM +0800, Hu Tao wrote:
> On Mon, Jun 16, 2014 at 02:09:33PM +0300, Michael S. Tsirkin wrote:
> > On Mon, Jun 16, 2014 at 06:05:40PM +0800, Hu Tao wrote:
> > > Michael,
> > > 
> > > This is fixes for your pci tree v2. please review. thanks!
> > 
> > Patches 1,4 applied. Thanks!
> 
> What about patches 2,3? If you need I can resend patch 2 based on Igor's
> comments.   About patch 3, I assume you leave it as it is not a
> regression.

Patch 2 isn't a regression either, right?
Please go ahead and repost patches 2,3 addressing
comments.
They are unrelated so don't have to be in a series, even.

> > 
> > > changes to v1:
> > > 
> > >   1. adds the missing query-memdev and change into querying qom for memdevs(patch 1)
> > >   2. tweak error message (patch 2)
> > >   3. improve error handling for memory-backend-file (patch 3)
> > >   4. fixup the problem that nodes not start from 0 (patch 4)
> > > 
> > > Hu Tao (4):
> > >   qmp: add query-memdev
> > >   check if we have space left for hotplugged memory
> > >   memory-backend-file: improve error handling
> > >   fixup! numa: add -numa node,memdev= option
> > > 
> > >  exec.c           | 26 ++++++++++++-----
> > >  hw/mem/pc-dimm.c |  7 +++++
> > >  numa.c           | 89 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > >  qapi-schema.json | 40 +++++++++++++++++++++++++
> > >  qmp-commands.hx  | 38 ++++++++++++++++++++++++
> > >  5 files changed, 191 insertions(+), 9 deletions(-)
> > > 
> > > -- 
> > > 1.9.3

Re: [Qemu-devel] [PATCH v2 3/4] memory-backend-file: improve error handling

[Michael S. Tsirkin]

On Tue, Jun 17, 2014 at 06:09:08PM +0800, Hu Tao wrote:
> On Tue, Jun 17, 2014 at 12:03:13PM +0200, Igor Mammedov wrote:
> > On Mon, 16 Jun 2014 18:05:43 +0800
> > Hu Tao <hutao@cn.fujitsu.com> wrote:
> > 
> > > This patch fixes two problems of memory-backend-file:
> > > 
> > > 1. If user adds a memory-backend-file object using object_add command,
> > >    specifying a non-existing directory for property mem-path, qemu
> > >    will core dump with message:
> > > 
> > >      /nonexistingdir: No such file or directory
> > >      Bad ram offset fffffffffffff000
> > >      Aborted (core dumped)
> > > 
> > > 2. If user adds a memory-backend-file object using object_add command,
> > >    specifying a size that is less than huge page size, qemu
> > >    will core dump with message:
> > > 
> > >      Bad ram offset fffffffffffff000
> > >      Aborted (core dumped)
> > > 
> > > Signed-off-by: Hu Tao <hutao@cn.fujitsu.com>
> > > ---
> > >  exec.c | 26 ++++++++++++++++++--------
> > >  1 file changed, 18 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/exec.c b/exec.c
> > > index 8705cc5..a6afb4d 100644
> > > --- a/exec.c
> > > +++ b/exec.c
> > [...]
> > > @@ -1308,7 +1311,14 @@ ram_addr_t qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> > >          return -1;
> > >      }
> > >  
> > > -    return ram_block_add(new_block);
> > > +    addr = ram_block_add(new_block);
> > > +    if (addr == -1) {
> > how this hunk is relevant to commit message?
> > 
> > and more important in what case ram_block_add() returns -1?
> 
> See patch 03 int v1(titled [PATCH RFC 3/4] exec: don't exit unconditionally if failed to allocate memory)
> It is not included in this version but we'll fix it after the merge.

So maybe defer this hunk to after the merge too.

> > 
> > > +        g_free(new_block);
> > > +        error_setg(errp, "failed to allocate memory\n");
> > > +        return -1;
> > > +    }
> > > +
> > > +    return addr;
> > >  }
> > >  #endif
> > >