Re: [Qemu-devel] [PATCH] numa: check for busy memory backend
On Mon, Jun 30, 2014 at 12:12:20PM +0300, Michael S. Tsirkin wrote: > On Mon, Jun 30, 2014 at 10:48:22AM +0200, Igor Mammedov wrote: > > On Mon, 30 Jun 2014 11:28:07 +0300 > > "Michael S. Tsirkin" wrote: > > > > > On Mon, Jun 30, 2014 at 03:46:56PM +0800, Hu Tao wrote: > > > > On Mon, Jun 30, 2014 at 09:53:20AM +0300, Michael S. Tsirkin wrote: > > > > > On Mon, Jun 30, 2014 at 01:33:42PM +0800, Hu Tao wrote: > > > > > > On Sun, Jun 29, 2014 at 06:20:22PM +0300, Michael S. Tsirkin wrote: > > > > > > > On Wed, Jun 25, 2014 at 05:04:14PM +0800, Hu Tao wrote: > > > > > > > > ..to prevent one memory backend from being used by more than > > > > > > > > one numa > > > > > > > > node. > > > > > > > > > > > > > > Thanks, but please always make the msg content self-contained > > > > > > > so it can be understood without the subject. > > > > > > > E.g. here, just drop "..to". > > > > > > > > > > > > > > Are you sure we want this? Is there a chance sharing a backend > > > > > > > can be useful? > > > > > > > > > > > > This patch is actually a bug fix. > > > > > > > > > > It is? What is the bug and how to reproduce it? > > > > > > > > If user specifies the same memory backend for two numa nodes: > > > > > > > > ./x86_64-softmmu/qemu-system-x86_64 -hda > > > > /home/data/libvirt-images/f18.img -m 512M \ > > > > -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm \ > > > > -object memory-backend-ram,size=256M,id=ram0 \ > > > > -numa node,nodeid=0,memdev=ram0 \ > > > > -numa node,nodeid=1,memdev=ram0 > > > > > > > > > I am not sure we should write a ton of code to validate qemu > > > > > configuration, as long as qemu does not assert. > > > > > > > > It seems qemu does not provide a way to disable assert currently. > > > > Even if I removed asserts on the code path in my test, there is another > > > > problem that it hits an infinite in render_memory_region(). > > > > > > OK so this is what commit log should say: > > > ---> > > > Specifying the same memory region twice leads to an assert: > > > > > > ./x86_64-softmmu/qemu-system-x86_64 -m 512M -enable-kvm -object > > > memory-backend-ram,size=256M,id=ram0 -numa node,nodeid=0,memdev=ram0 > > > -numa node,nodeid=1,memdev=ram0 > > > qemu-system-x86_64: /scm/qemu/memory.c:1506: > > > memory_region_add_subregion_common: Assertion `!subregion->container' > > > failed. > > > Aborted (core dumped) > > > > > > Detect and exit with an error message instead. > > > <--- > > with fixed-up commit message: > > Reviewed-by: Igor Mammedov > > Sorry I want the error message fixed up too. Yes your error message is more clear. I'll send v2. Thanks for review. Regards, Hu > > > > > > > See? Explain why your patch makes sense, don't just repeat what it does. > > > > > > > > > > > > > > Even if we will want backend sharing, we > > > > > > can do it after. > > > > > > > > > > By reverting this patch? So why merge it? > > > > > > > > The point is qemu doesn't fire a bug no matter what user inputs. > > > > > > > > > > > > > > > > > > > > > > > Igor, what's your take? > > > > > > > > > > > > > > > > > > > > > > > Signed-off-by: Hu Tao > > > > > > > > --- > > > > > > > > numa.c | 7 +++ > > > > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > > > > > > > diff --git a/numa.c b/numa.c > > > > > > > > index e471afe..6c1c554 100644 > > > > > > > > --- a/numa.c > > > > > > > > +++ b/numa.c > > > > > > > > @@ -279,6 +279,13 @@ void > > > > > > > > memory_region_allocate_system_memory(MemoryRegion *mr, Object > > > > > > > > *owner, > > > > > > > > exit(1); > > > > > > > > } > > > > > > > > > > > > > > > > +if (memory_region_is_mapped(seg)) { > > > > > > > > +char *path = > > > > > > > > object_get_canonical_path_component(OBJECT(backend)); > > > > > > > > +error_report("memory backend %s is busy", path); > > > > > > That's not very clear. How about: > > > memory backend %s is used multiple times. Each -numa option must use a > > > different memdev value. > > > > > > > > > > > +g_free(path); > > > > > > As we are going to exit anyway, it does not make sense to bother with > > > this. > > > > > > > > > > > +exit(1); > > > > > > > > +} > > > > > > > > + > > > > > > > > memory_region_add_subregion(mr, addr, seg); > > > > > > > > vmstate_register_ram_global(seg); > > > > > > > > addr += size; > > > > > > > > -- > > > > > > > > 1.9.3 > > >
Re: [Qemu-devel] [PATCH] numa: check for busy memory backend
On Mon, Jun 30, 2014 at 10:48:22AM +0200, Igor Mammedov wrote: > On Mon, 30 Jun 2014 11:28:07 +0300 > "Michael S. Tsirkin" wrote: > > > On Mon, Jun 30, 2014 at 03:46:56PM +0800, Hu Tao wrote: > > > On Mon, Jun 30, 2014 at 09:53:20AM +0300, Michael S. Tsirkin wrote: > > > > On Mon, Jun 30, 2014 at 01:33:42PM +0800, Hu Tao wrote: > > > > > On Sun, Jun 29, 2014 at 06:20:22PM +0300, Michael S. Tsirkin wrote: > > > > > > On Wed, Jun 25, 2014 at 05:04:14PM +0800, Hu Tao wrote: > > > > > > > ..to prevent one memory backend from being used by more than one > > > > > > > numa > > > > > > > node. > > > > > > > > > > > > Thanks, but please always make the msg content self-contained > > > > > > so it can be understood without the subject. > > > > > > E.g. here, just drop "..to". > > > > > > > > > > > > Are you sure we want this? Is there a chance sharing a backend > > > > > > can be useful? > > > > > > > > > > This patch is actually a bug fix. > > > > > > > > It is? What is the bug and how to reproduce it? > > > > > > If user specifies the same memory backend for two numa nodes: > > > > > > ./x86_64-softmmu/qemu-system-x86_64 -hda > > > /home/data/libvirt-images/f18.img -m 512M \ > > > -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm \ > > > -object memory-backend-ram,size=256M,id=ram0 \ > > > -numa node,nodeid=0,memdev=ram0 \ > > > -numa node,nodeid=1,memdev=ram0 > > > > > > > I am not sure we should write a ton of code to validate qemu > > > > configuration, as long as qemu does not assert. > > > > > > It seems qemu does not provide a way to disable assert currently. > > > Even if I removed asserts on the code path in my test, there is another > > > problem that it hits an infinite in render_memory_region(). > > > > OK so this is what commit log should say: > > ---> > > Specifying the same memory region twice leads to an assert: > > > > ./x86_64-softmmu/qemu-system-x86_64 -m 512M -enable-kvm -object > > memory-backend-ram,size=256M,id=ram0 -numa node,nodeid=0,memdev=ram0 > > -numa node,nodeid=1,memdev=ram0 > > qemu-system-x86_64: /scm/qemu/memory.c:1506: > > memory_region_add_subregion_common: Assertion `!subregion->container' > > failed. > > Aborted (core dumped) > > > > Detect and exit with an error message instead. > > <--- > with fixed-up commit message: > Reviewed-by: Igor Mammedov Sorry I want the error message fixed up too. > > > > See? Explain why your patch makes sense, don't just repeat what it does. > > > > > > > > > > > Even if we will want backend sharing, we > > > > > can do it after. > > > > > > > > By reverting this patch? So why merge it? > > > > > > The point is qemu doesn't fire a bug no matter what user inputs. > > > > > > > > > > > > > > > > > > > Igor, what's your take? > > > > > > > > > > > > > > > > > > > > Signed-off-by: Hu Tao > > > > > > > --- > > > > > > > numa.c | 7 +++ > > > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > > > > > diff --git a/numa.c b/numa.c > > > > > > > index e471afe..6c1c554 100644 > > > > > > > --- a/numa.c > > > > > > > +++ b/numa.c > > > > > > > @@ -279,6 +279,13 @@ void > > > > > > > memory_region_allocate_system_memory(MemoryRegion *mr, Object > > > > > > > *owner, > > > > > > > exit(1); > > > > > > > } > > > > > > > > > > > > > > +if (memory_region_is_mapped(seg)) { > > > > > > > +char *path = > > > > > > > object_get_canonical_path_component(OBJECT(backend)); > > > > > > > +error_report("memory backend %s is busy", path); > > > > That's not very clear. How about: > > memory backend %s is used multiple times. Each -numa option must use a > > different memdev value. > > > > > > > > > +g_free(path); > > > > As we are going to exit anyway, it does not make sense to bother with this. > > > > > > > > > +exit(1); > > > > > > > +} > > > > > > > + > > > > > > > memory_region_add_subregion(mr, addr, seg); > > > > > > > vmstate_register_ram_global(seg); > > > > > > > addr += size; > > > > > > > -- > > > > > > > 1.9.3 > >
Re: [Qemu-devel] [PATCH] numa: check for busy memory backend
On Mon, 30 Jun 2014 11:28:07 +0300 "Michael S. Tsirkin" wrote: > On Mon, Jun 30, 2014 at 03:46:56PM +0800, Hu Tao wrote: > > On Mon, Jun 30, 2014 at 09:53:20AM +0300, Michael S. Tsirkin wrote: > > > On Mon, Jun 30, 2014 at 01:33:42PM +0800, Hu Tao wrote: > > > > On Sun, Jun 29, 2014 at 06:20:22PM +0300, Michael S. Tsirkin wrote: > > > > > On Wed, Jun 25, 2014 at 05:04:14PM +0800, Hu Tao wrote: > > > > > > ..to prevent one memory backend from being used by more than one > > > > > > numa > > > > > > node. > > > > > > > > > > Thanks, but please always make the msg content self-contained > > > > > so it can be understood without the subject. > > > > > E.g. here, just drop "..to". > > > > > > > > > > Are you sure we want this? Is there a chance sharing a backend > > > > > can be useful? > > > > > > > > This patch is actually a bug fix. > > > > > > It is? What is the bug and how to reproduce it? > > > > If user specifies the same memory backend for two numa nodes: > > > > ./x86_64-softmmu/qemu-system-x86_64 -hda /home/data/libvirt-images/f18.img > > -m 512M \ > > -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm \ > > -object memory-backend-ram,size=256M,id=ram0 \ > > -numa node,nodeid=0,memdev=ram0 \ > > -numa node,nodeid=1,memdev=ram0 > > > > > I am not sure we should write a ton of code to validate qemu > > > configuration, as long as qemu does not assert. > > > > It seems qemu does not provide a way to disable assert currently. > > Even if I removed asserts on the code path in my test, there is another > > problem that it hits an infinite in render_memory_region(). > > OK so this is what commit log should say: > ---> > Specifying the same memory region twice leads to an assert: > > ./x86_64-softmmu/qemu-system-x86_64 -m 512M -enable-kvm -object > memory-backend-ram,size=256M,id=ram0 -numa node,nodeid=0,memdev=ram0 > -numa node,nodeid=1,memdev=ram0 > qemu-system-x86_64: /scm/qemu/memory.c:1506: > memory_region_add_subregion_common: Assertion `!subregion->container' > failed. > Aborted (core dumped) > > Detect and exit with an error message instead. > <--- with fixed-up commit message: Reviewed-by: Igor Mammedov > > See? Explain why your patch makes sense, don't just repeat what it does. > > > > > > > > Even if we will want backend sharing, we > > > > can do it after. > > > > > > By reverting this patch? So why merge it? > > > > The point is qemu doesn't fire a bug no matter what user inputs. > > > > > > > > > > > > > > > Igor, what's your take? > > > > > > > > > > > > > > > > > Signed-off-by: Hu Tao > > > > > > --- > > > > > > numa.c | 7 +++ > > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > > > diff --git a/numa.c b/numa.c > > > > > > index e471afe..6c1c554 100644 > > > > > > --- a/numa.c > > > > > > +++ b/numa.c > > > > > > @@ -279,6 +279,13 @@ void > > > > > > memory_region_allocate_system_memory(MemoryRegion *mr, Object > > > > > > *owner, > > > > > > exit(1); > > > > > > } > > > > > > > > > > > > +if (memory_region_is_mapped(seg)) { > > > > > > +char *path = > > > > > > object_get_canonical_path_component(OBJECT(backend)); > > > > > > +error_report("memory backend %s is busy", path); > > That's not very clear. How about: > memory backend %s is used multiple times. Each -numa option must use a > different memdev value. > > > > > > > +g_free(path); > > As we are going to exit anyway, it does not make sense to bother with this. > > > > > > > +exit(1); > > > > > > +} > > > > > > + > > > > > > memory_region_add_subregion(mr, addr, seg); > > > > > > vmstate_register_ram_global(seg); > > > > > > addr += size; > > > > > > -- > > > > > > 1.9.3 >
Re: [Qemu-devel] [PATCH] numa: check for busy memory backend
On Mon, Jun 30, 2014 at 03:46:56PM +0800, Hu Tao wrote: > On Mon, Jun 30, 2014 at 09:53:20AM +0300, Michael S. Tsirkin wrote: > > On Mon, Jun 30, 2014 at 01:33:42PM +0800, Hu Tao wrote: > > > On Sun, Jun 29, 2014 at 06:20:22PM +0300, Michael S. Tsirkin wrote: > > > > On Wed, Jun 25, 2014 at 05:04:14PM +0800, Hu Tao wrote: > > > > > ..to prevent one memory backend from being used by more than one numa > > > > > node. > > > > > > > > Thanks, but please always make the msg content self-contained > > > > so it can be understood without the subject. > > > > E.g. here, just drop "..to". > > > > > > > > Are you sure we want this? Is there a chance sharing a backend > > > > can be useful? > > > > > > This patch is actually a bug fix. > > > > It is? What is the bug and how to reproduce it? > > If user specifies the same memory backend for two numa nodes: > > ./x86_64-softmmu/qemu-system-x86_64 -hda /home/data/libvirt-images/f18.img > -m 512M \ > -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm \ > -object memory-backend-ram,size=256M,id=ram0 \ > -numa node,nodeid=0,memdev=ram0 \ > -numa node,nodeid=1,memdev=ram0 > > > I am not sure we should write a ton of code to validate qemu > > configuration, as long as qemu does not assert. > > It seems qemu does not provide a way to disable assert currently. > Even if I removed asserts on the code path in my test, there is another > problem that it hits an infinite in render_memory_region(). OK so this is what commit log should say: ---> Specifying the same memory region twice leads to an assert: ./x86_64-softmmu/qemu-system-x86_64 -m 512M -enable-kvm -object memory-backend-ram,size=256M,id=ram0 -numa node,nodeid=0,memdev=ram0 -numa node,nodeid=1,memdev=ram0 qemu-system-x86_64: /scm/qemu/memory.c:1506: memory_region_add_subregion_common: Assertion `!subregion->container' failed. Aborted (core dumped) Detect and exit with an error message instead. <--- See? Explain why your patch makes sense, don't just repeat what it does. > > > > > Even if we will want backend sharing, we > > > can do it after. > > > > By reverting this patch? So why merge it? > > The point is qemu doesn't fire a bug no matter what user inputs. > > > > > > > > > > > Igor, what's your take? > > > > > > > > > > > > > > Signed-off-by: Hu Tao > > > > > --- > > > > > numa.c | 7 +++ > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > diff --git a/numa.c b/numa.c > > > > > index e471afe..6c1c554 100644 > > > > > --- a/numa.c > > > > > +++ b/numa.c > > > > > @@ -279,6 +279,13 @@ void > > > > > memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, > > > > > exit(1); > > > > > } > > > > > > > > > > +if (memory_region_is_mapped(seg)) { > > > > > +char *path = > > > > > object_get_canonical_path_component(OBJECT(backend)); > > > > > +error_report("memory backend %s is busy", path); That's not very clear. How about: memory backend %s is used multiple times. Each -numa option must use a different memdev value. > > > > > +g_free(path); As we are going to exit anyway, it does not make sense to bother with this. > > > > > +exit(1); > > > > > +} > > > > > + > > > > > memory_region_add_subregion(mr, addr, seg); > > > > > vmstate_register_ram_global(seg); > > > > > addr += size; > > > > > -- > > > > > 1.9.3
Re: [Qemu-devel] [PATCH] numa: check for busy memory backend
On Mon, Jun 30, 2014 at 09:53:20AM +0300, Michael S. Tsirkin wrote: > On Mon, Jun 30, 2014 at 01:33:42PM +0800, Hu Tao wrote: > > On Sun, Jun 29, 2014 at 06:20:22PM +0300, Michael S. Tsirkin wrote: > > > On Wed, Jun 25, 2014 at 05:04:14PM +0800, Hu Tao wrote: > > > > ..to prevent one memory backend from being used by more than one numa > > > > node. > > > > > > Thanks, but please always make the msg content self-contained > > > so it can be understood without the subject. > > > E.g. here, just drop "..to". > > > > > > Are you sure we want this? Is there a chance sharing a backend > > > can be useful? > > > > This patch is actually a bug fix. > > It is? What is the bug and how to reproduce it? If user specifies the same memory backend for two numa nodes: ./x86_64-softmmu/qemu-system-x86_64 -hda /home/data/libvirt-images/f18.img -m 512M \ -qmp unix:/tmp/m,server,nowait -monitor stdio -enable-kvm \ -object memory-backend-ram,size=256M,id=ram0 \ -numa node,nodeid=0,memdev=ram0 \ -numa node,nodeid=1,memdev=ram0 > I am not sure we should write a ton of code to validate qemu > configuration, as long as qemu does not assert. It seems qemu does not provide a way to disable assert currently. Even if I removed asserts on the code path in my test, there is another problem that it hits an infinite in render_memory_region(). > > > Even if we will want backend sharing, we > > can do it after. > > By reverting this patch? So why merge it? The point is qemu doesn't fire a bug no matter what user inputs. > > > > > > > Igor, what's your take? > > > > > > > > > > > Signed-off-by: Hu Tao > > > > --- > > > > numa.c | 7 +++ > > > > 1 file changed, 7 insertions(+) > > > > > > > > diff --git a/numa.c b/numa.c > > > > index e471afe..6c1c554 100644 > > > > --- a/numa.c > > > > +++ b/numa.c > > > > @@ -279,6 +279,13 @@ void > > > > memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, > > > > exit(1); > > > > } > > > > > > > > +if (memory_region_is_mapped(seg)) { > > > > +char *path = > > > > object_get_canonical_path_component(OBJECT(backend)); > > > > +error_report("memory backend %s is busy", path); > > > > +g_free(path); > > > > +exit(1); > > > > +} > > > > + > > > > memory_region_add_subregion(mr, addr, seg); > > > > vmstate_register_ram_global(seg); > > > > addr += size; > > > > -- > > > > 1.9.3
Re: [Qemu-devel] [PATCH] numa: check for busy memory backend
On Mon, Jun 30, 2014 at 01:33:42PM +0800, Hu Tao wrote: > On Sun, Jun 29, 2014 at 06:20:22PM +0300, Michael S. Tsirkin wrote: > > On Wed, Jun 25, 2014 at 05:04:14PM +0800, Hu Tao wrote: > > > ..to prevent one memory backend from being used by more than one numa > > > node. > > > > Thanks, but please always make the msg content self-contained > > so it can be understood without the subject. > > E.g. here, just drop "..to". > > > > Are you sure we want this? Is there a chance sharing a backend > > can be useful? > > This patch is actually a bug fix. It is? What is the bug and how to reproduce it? I am not sure we should write a ton of code to validate qemu configuration, as long as qemu does not assert. > Even if we will want backend sharing, we > can do it after. By reverting this patch? So why merge it? > > > > Igor, what's your take? > > > > > > > > Signed-off-by: Hu Tao > > > --- > > > numa.c | 7 +++ > > > 1 file changed, 7 insertions(+) > > > > > > diff --git a/numa.c b/numa.c > > > index e471afe..6c1c554 100644 > > > --- a/numa.c > > > +++ b/numa.c > > > @@ -279,6 +279,13 @@ void > > > memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, > > > exit(1); > > > } > > > > > > +if (memory_region_is_mapped(seg)) { > > > +char *path = > > > object_get_canonical_path_component(OBJECT(backend)); > > > +error_report("memory backend %s is busy", path); > > > +g_free(path); > > > +exit(1); > > > +} > > > + > > > memory_region_add_subregion(mr, addr, seg); > > > vmstate_register_ram_global(seg); > > > addr += size; > > > -- > > > 1.9.3
Re: [Qemu-devel] [PATCH] numa: check for busy memory backend
On Sun, Jun 29, 2014 at 06:20:22PM +0300, Michael S. Tsirkin wrote: > On Wed, Jun 25, 2014 at 05:04:14PM +0800, Hu Tao wrote: > > ..to prevent one memory backend from being used by more than one numa > > node. > > Thanks, but please always make the msg content self-contained > so it can be understood without the subject. > E.g. here, just drop "..to". > > Are you sure we want this? Is there a chance sharing a backend > can be useful? This patch is actually a bug fix. Even if we will want backend sharing, we can do it after. > > Igor, what's your take? > > > > > Signed-off-by: Hu Tao > > --- > > numa.c | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git a/numa.c b/numa.c > > index e471afe..6c1c554 100644 > > --- a/numa.c > > +++ b/numa.c > > @@ -279,6 +279,13 @@ void memory_region_allocate_system_memory(MemoryRegion > > *mr, Object *owner, > > exit(1); > > } > > > > +if (memory_region_is_mapped(seg)) { > > +char *path = > > object_get_canonical_path_component(OBJECT(backend)); > > +error_report("memory backend %s is busy", path); > > +g_free(path); > > +exit(1); > > +} > > + > > memory_region_add_subregion(mr, addr, seg); > > vmstate_register_ram_global(seg); > > addr += size; > > -- > > 1.9.3
Re: [Qemu-devel] [PATCH] numa: check for busy memory backend
On Wed, Jun 25, 2014 at 05:04:14PM +0800, Hu Tao wrote: > ..to prevent one memory backend from being used by more than one numa > node. Thanks, but please always make the msg content self-contained so it can be understood without the subject. E.g. here, just drop "..to". Are you sure we want this? Is there a chance sharing a backend can be useful? Igor, what's your take? > > Signed-off-by: Hu Tao > --- > numa.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/numa.c b/numa.c > index e471afe..6c1c554 100644 > --- a/numa.c > +++ b/numa.c > @@ -279,6 +279,13 @@ void memory_region_allocate_system_memory(MemoryRegion > *mr, Object *owner, > exit(1); > } > > +if (memory_region_is_mapped(seg)) { > +char *path = > object_get_canonical_path_component(OBJECT(backend)); > +error_report("memory backend %s is busy", path); > +g_free(path); > +exit(1); > +} > + > memory_region_add_subregion(mr, addr, seg); > vmstate_register_ram_global(seg); > addr += size; > -- > 1.9.3
Re: [Qemu-devel] [PATCH] numa: check for busy memory backend
Igor Mammedov writes: > On Wed, 25 Jun 2014 17:04:14 +0800 > Hu Tao wrote: > >> ..to prevent one memory backend from being used by more than one numa >> node. >> >> Signed-off-by: Hu Tao >> --- >> numa.c | 7 +++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/numa.c b/numa.c >> index e471afe..6c1c554 100644 >> --- a/numa.c >> +++ b/numa.c >> @@ -279,6 +279,13 @@ void memory_region_allocate_system_memory(MemoryRegion >> *mr, Object *owner, >> exit(1); >> } >> >> +if (memory_region_is_mapped(seg)) { >> +char *path = >> object_get_canonical_path_component(OBJECT(backend)); >> +error_report("memory backend %s is busy", path); >> +g_free(path); >> +exit(1); > s/1/EXIT_FAILURE/ please I count >600 instances of exit() with a numeric argument (not counting some 50 sys.exit() in Python code), but less than 40 using EXIT_SUCCESS or EXIT_FAILURE. The abstraction provided by EXIT_SUCCESS / EXIT_FAILURE is basically worthless anyway. > >> +} >> + >> memory_region_add_subregion(mr, addr, seg); >> vmstate_register_ram_global(seg); >> addr += size;
Re: [Qemu-devel] [PATCH] numa: check for busy memory backend
On Wed, 25 Jun 2014 17:04:14 +0800 Hu Tao wrote: > ..to prevent one memory backend from being used by more than one numa > node. > > Signed-off-by: Hu Tao > --- > numa.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/numa.c b/numa.c > index e471afe..6c1c554 100644 > --- a/numa.c > +++ b/numa.c > @@ -279,6 +279,13 @@ void memory_region_allocate_system_memory(MemoryRegion > *mr, Object *owner, > exit(1); > } > > +if (memory_region_is_mapped(seg)) { > +char *path = > object_get_canonical_path_component(OBJECT(backend)); > +error_report("memory backend %s is busy", path); > +g_free(path); > +exit(1); s/1/EXIT_FAILURE/ please > +} > + > memory_region_add_subregion(mr, addr, seg); > vmstate_register_ram_global(seg); > addr += size;
[Qemu-devel] [PATCH] numa: check for busy memory backend
..to prevent one memory backend from being used by more than one numa node. Signed-off-by: Hu Tao --- numa.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/numa.c b/numa.c index e471afe..6c1c554 100644 --- a/numa.c +++ b/numa.c @@ -279,6 +279,13 @@ void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner, exit(1); } +if (memory_region_is_mapped(seg)) { +char *path = object_get_canonical_path_component(OBJECT(backend)); +error_report("memory backend %s is busy", path); +g_free(path); +exit(1); +} + memory_region_add_subregion(mr, addr, seg); vmstate_register_ram_global(seg); addr += size; -- 1.9.3