[Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Igor Mammedov]

When adding hostmem backend at runtime, QEMU might exit with error:
  "os_mem_prealloc: Insufficient free host memory pages available to allocate guest RAM"

It happens due to os_mem_prealloc() not handling errors gracefully.

Fix it by passing errp argument so that os_mem_prealloc() could
report error to callers and undo performed allocation when
os_mem_prealloc() fails.

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 include/qemu/osdep.h |  2 +-
 backends/hostmem.c   | 18 ++++++++++++++----
 exec.c               | 10 ++++++++--
 util/oslib-posix.c   | 25 ++++++++++++-------------
 util/oslib-win32.c   |  2 +-
 5 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index fbb8759..d7c111d 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -379,7 +379,7 @@ unsigned long qemu_getauxval(unsigned long type);
 
 void qemu_set_tty_echo(int fd, bool echo);
 
-void os_mem_prealloc(int fd, char *area, size_t sz);
+void os_mem_prealloc(int fd, char *area, size_t sz, Error **errp);
 
 int qemu_read_password(char *buf, int buf_size);
 
diff --git a/backends/hostmem.c b/backends/hostmem.c
index ac80257..b7a208d 100644
--- a/backends/hostmem.c
+++ b/backends/hostmem.c
@@ -203,6 +203,7 @@ static bool host_memory_backend_get_prealloc(Object *obj, Error **errp)
 static void host_memory_backend_set_prealloc(Object *obj, bool value,
                                              Error **errp)
 {
+    Error *local_err = NULL;
     HostMemoryBackend *backend = MEMORY_BACKEND(obj);
 
     if (backend->force_prealloc) {
@@ -223,7 +224,11 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
         void *ptr = memory_region_get_ram_ptr(&backend->mr);
         uint64_t sz = memory_region_size(&backend->mr);
 
-        os_mem_prealloc(fd, ptr, sz);
+        os_mem_prealloc(fd, ptr, sz, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
         backend->prealloc = true;
     }
 }
@@ -286,8 +291,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
     if (bc->alloc) {
         bc->alloc(backend, &local_err);
         if (local_err) {
-            error_propagate(errp, local_err);
-            return;
+            goto out;
         }
 
         ptr = memory_region_get_ram_ptr(&backend->mr);
@@ -343,9 +347,15 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
          * specified NUMA policy in place.
          */
         if (backend->prealloc) {
-            os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz);
+            os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz,
+                            &local_err);
+            if (local_err) {
+                goto out;
+            }
         }
     }
+out:
+    error_propagate(errp, local_err);
 }
 
 static bool
diff --git a/exec.c b/exec.c
index 60cf46a..bf1446c 100644
--- a/exec.c
+++ b/exec.c
@@ -1268,7 +1268,7 @@ static void *file_ram_alloc(RAMBlock *block,
     char *filename;
     char *sanitized_name;
     char *c;
-    void *area;
+    void *area = MAP_FAILED;
     int fd = -1;
     int64_t page_size;
 
@@ -1356,13 +1356,19 @@ static void *file_ram_alloc(RAMBlock *block,
     }
 
     if (mem_prealloc) {
-        os_mem_prealloc(fd, area, memory);
+        os_mem_prealloc(fd, area, memory, errp);
+        if (errp && *errp) {
+            goto error;
+        }
     }
 
     block->fd = fd;
     return area;
 
 error:
+    if (area != MAP_FAILED) {
+        qemu_ram_munmap(area, memory);
+    }
     if (unlink_on_error) {
         unlink(path);
     }
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 6d70d9a..a60ac97 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -318,7 +318,7 @@ static void sigbus_handler(int signal)
     siglongjmp(sigjump, 1);
 }
 
-void os_mem_prealloc(int fd, char *area, size_t memory)
+void os_mem_prealloc(int fd, char *area, size_t memory, Error **errp)
 {
     int ret;
     struct sigaction act, oldact;
@@ -330,8 +330,9 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
 
     ret = sigaction(SIGBUS, &act, &oldact);
     if (ret) {
-        perror("os_mem_prealloc: failed to install signal handler");
-        exit(1);
+        error_setg_errno(errp, errno,
+            "os_mem_prealloc: failed to install signal handler");
+        return;
     }
 
     /* unblock SIGBUS */
@@ -340,9 +341,8 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
     pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
 
     if (sigsetjmp(sigjump, 1)) {
-        fprintf(stderr, "os_mem_prealloc: Insufficient free host memory "
-                        "pages available to allocate guest RAM\n");
-        exit(1);
+        error_setg(errp, "os_mem_prealloc: Insufficient free host memory "
+            "pages available to allocate guest RAM\n");
     } else {
         int i;
         size_t hpagesize = qemu_fd_getpagesize(fd);
@@ -352,15 +352,14 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
         for (i = 0; i < numpages; i++) {
             memset(area + (hpagesize * i), 0, 1);
         }
+    }
 
-        ret = sigaction(SIGBUS, &oldact, NULL);
-        if (ret) {
-            perror("os_mem_prealloc: failed to reinstall signal handler");
-            exit(1);
-        }
-
-        pthread_sigmask(SIG_SETMASK, &oldset, NULL);
+    ret = sigaction(SIGBUS, &oldact, NULL);
+    if (ret) {
+        perror("os_mem_prealloc: failed to reinstall signal handler");
+        exit(1);
     }
+    pthread_sigmask(SIG_SETMASK, &oldset, NULL);
 }
 
 
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 6debc2b..4c1dcf1 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -539,7 +539,7 @@ int getpagesize(void)
     return system_info.dwPageSize;
 }
 
-void os_mem_prealloc(int fd, char *area, size_t memory)
+void os_mem_prealloc(int fd, char *area, size_t memory, Error **errp)
 {
     int i;
     size_t pagesize = getpagesize();
-- 
2.7.4

Re: [Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Markus Armbruster]

Igor Mammedov <imammedo@redhat.com> writes:

> When adding hostmem backend at runtime, QEMU might exit with error:
>   "os_mem_prealloc: Insufficient free host memory pages available to allocate guest RAM"
>
> It happens due to os_mem_prealloc() not handling errors gracefully.
>
> Fix it by passing errp argument so that os_mem_prealloc() could
> report error to callers and undo performed allocation when
> os_mem_prealloc() fails.
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/qemu/osdep.h |  2 +-
>  backends/hostmem.c   | 18 ++++++++++++++----
>  exec.c               | 10 ++++++++--
>  util/oslib-posix.c   | 25 ++++++++++++-------------
>  util/oslib-win32.c   |  2 +-
>  5 files changed, 36 insertions(+), 21 deletions(-)
>
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index fbb8759..d7c111d 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -379,7 +379,7 @@ unsigned long qemu_getauxval(unsigned long type);
>  
>  void qemu_set_tty_echo(int fd, bool echo);
>  
> -void os_mem_prealloc(int fd, char *area, size_t sz);
> +void os_mem_prealloc(int fd, char *area, size_t sz, Error **errp);
>  
>  int qemu_read_password(char *buf, int buf_size);
>  
> diff --git a/backends/hostmem.c b/backends/hostmem.c
> index ac80257..b7a208d 100644
> --- a/backends/hostmem.c
> +++ b/backends/hostmem.c
> @@ -203,6 +203,7 @@ static bool host_memory_backend_get_prealloc(Object *obj, Error **errp)
>  static void host_memory_backend_set_prealloc(Object *obj, bool value,
>                                               Error **errp)
>  {
> +    Error *local_err = NULL;
>      HostMemoryBackend *backend = MEMORY_BACKEND(obj);
>  
>      if (backend->force_prealloc) {
> @@ -223,7 +224,11 @@ static void host_memory_backend_set_prealloc(Object *obj, bool value,
>          void *ptr = memory_region_get_ram_ptr(&backend->mr);
>          uint64_t sz = memory_region_size(&backend->mr);
>  
> -        os_mem_prealloc(fd, ptr, sz);
> +        os_mem_prealloc(fd, ptr, sz, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
>          backend->prealloc = true;
>      }
>  }
> @@ -286,8 +291,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>      if (bc->alloc) {
>          bc->alloc(backend, &local_err);
>          if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> +            goto out;

I'd leave the tail merging optimization to the compiler, i.e. don't
touch the code here, and ...

>          }
>  
>          ptr = memory_region_get_ram_ptr(&backend->mr);
> @@ -343,9 +347,15 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
>           * specified NUMA policy in place.
>           */
>          if (backend->prealloc) {
> -            os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz);
> +            os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz,
> +                            &local_err);
> +            if (local_err) {
> +                goto out;

... have the obvious error_propagate() + return here.  Matter of taste,
between you and the maintainer.  Except there is none.  Inexcusable for
a file created in 2014.  Suggest you appoint yourself.

> +            }
>          }
>      }
> +out:
> +    error_propagate(errp, local_err);
>  }
>  
>  static bool
> diff --git a/exec.c b/exec.c
> index 60cf46a..bf1446c 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -1268,7 +1268,7 @@ static void *file_ram_alloc(RAMBlock *block,
>      char *filename;
>      char *sanitized_name;
>      char *c;
> -    void *area;
> +    void *area = MAP_FAILED;
>      int fd = -1;
>      int64_t page_size;
>  
> @@ -1356,13 +1356,19 @@ static void *file_ram_alloc(RAMBlock *block,
>      }
>  
>      if (mem_prealloc) {
> -        os_mem_prealloc(fd, area, memory);
> +        os_mem_prealloc(fd, area, memory, errp);
> +        if (errp && *errp) {
> +            goto error;
> +        }
>      }
>  
>      block->fd = fd;
>      return area;
>  
>  error:
> +    if (area != MAP_FAILED) {
> +        qemu_ram_munmap(area, memory);
> +    }
>      if (unlink_on_error) {
>          unlink(path);
>      }
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 6d70d9a..a60ac97 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -318,7 +318,7 @@ static void sigbus_handler(int signal)
>      siglongjmp(sigjump, 1);
>  }
>  
> -void os_mem_prealloc(int fd, char *area, size_t memory)
> +void os_mem_prealloc(int fd, char *area, size_t memory, Error **errp)
>  {
>      int ret;
>      struct sigaction act, oldact;
> @@ -330,8 +330,9 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
>  
>      ret = sigaction(SIGBUS, &act, &oldact);
>      if (ret) {
> -        perror("os_mem_prealloc: failed to install signal handler");
> -        exit(1);
> +        error_setg_errno(errp, errno,
> +            "os_mem_prealloc: failed to install signal handler");
> +        return;
>      }
>  
>      /* unblock SIGBUS */
> @@ -340,9 +341,8 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
>      pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
>  
>      if (sigsetjmp(sigjump, 1)) {
> -        fprintf(stderr, "os_mem_prealloc: Insufficient free host memory "
> -                        "pages available to allocate guest RAM\n");
> -        exit(1);
> +        error_setg(errp, "os_mem_prealloc: Insufficient free host memory "
> +            "pages available to allocate guest RAM\n");
>      } else {
>          int i;
>          size_t hpagesize = qemu_fd_getpagesize(fd);
> @@ -352,15 +352,14 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
>          for (i = 0; i < numpages; i++) {
>              memset(area + (hpagesize * i), 0, 1);
>          }
> +    }
>  
> -        ret = sigaction(SIGBUS, &oldact, NULL);
> -        if (ret) {
> -            perror("os_mem_prealloc: failed to reinstall signal handler");
> -            exit(1);
> -        }
> -
> -        pthread_sigmask(SIG_SETMASK, &oldset, NULL);
> +    ret = sigaction(SIGBUS, &oldact, NULL);
> +    if (ret) {
> +        perror("os_mem_prealloc: failed to reinstall signal handler");
> +        exit(1);

I guess you're keeping the exit() here, because you can't recover
cleanly from this error.  I should never happen anyway.  Worth a
comment, I think.

>      }
> +    pthread_sigmask(SIG_SETMASK, &oldset, NULL);
>  }
>  
>  
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 6debc2b..4c1dcf1 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -539,7 +539,7 @@ int getpagesize(void)
>      return system_info.dwPageSize;
>  }
>  
> -void os_mem_prealloc(int fd, char *area, size_t memory)
> +void os_mem_prealloc(int fd, char *area, size_t memory, Error **errp)
>  {
>      int i;
>      size_t pagesize = getpagesize();

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

Re: [Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Paolo Bonzini]

On 21/07/2016 10:36, Markus Armbruster wrote:
> ... have the obvious error_propagate() + return here.  Matter of taste,
> between you and the maintainer.  Except there is none.  Inexcusable for
> a file created in 2014.  Suggest you appoint yourself.

For now I've queued the patch, but I suggest that memory backends go
together with numa.c.

Paolo

Re: [Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Igor Mammedov]

On Thu, 21 Jul 2016 11:05:05 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> On 21/07/2016 10:36, Markus Armbruster wrote:
> > ... have the obvious error_propagate() + return here.  Matter of taste,
> > between you and the maintainer.  Except there is none.  Inexcusable for
> > a file created in 2014.  Suggest you appoint yourself.  
> 
> For now I've queued the patch
thanks

>, but I suggest that memory backends go
> together with numa.c.
Do you mean via Eduardo tree since here is numa maintainer?

> 
> Paolo
> 

Re: [Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Igor Mammedov]

On Thu, 21 Jul 2016 10:36:40 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Igor Mammedov <imammedo@redhat.com> writes:
[...]
> > @@ -286,8 +291,7 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
> >      if (bc->alloc) {
> >          bc->alloc(backend, &local_err);
> >          if (local_err) {
> > -            error_propagate(errp, local_err);
> > -            return;
> > +            goto out;  
> 
> I'd leave the tail merging optimization to the compiler, i.e. don't
> touch the code here, and ...
> 
> >          }
> >  
> >          ptr = memory_region_get_ram_ptr(&backend->mr);
> > @@ -343,9 +347,15 @@ host_memory_backend_memory_complete(UserCreatable *uc, Error **errp)
> >           * specified NUMA policy in place.
> >           */
> >          if (backend->prealloc) {
> > -            os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz);
> > +            os_mem_prealloc(memory_region_get_fd(&backend->mr), ptr, sz,
> > +                            &local_err);
> > +            if (local_err) {
> > +                goto out;  
> 
> ... have the obvious error_propagate() + return here.  Matter of taste,
> between you and the maintainer.  Except there is none.  Inexcusable for
> a file created in 2014.  Suggest you appoint yourself.
> 
> > +            }
I might if Eduardo declines or I could be and additional one.

wrt consolidating error_propagate(), it's what we were doing
in target-i386/cpu.c any time we touched similar code pathes
to reduce code duplication.

> >          }
> >      }
> > +out:
> > +    error_propagate(errp, local_err);
> >  }
[...]

> > @@ -352,15 +352,14 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
> >          for (i = 0; i < numpages; i++) {
> >              memset(area + (hpagesize * i), 0, 1);
> >          }
> > +    }
> >  
> > -        ret = sigaction(SIGBUS, &oldact, NULL);
> > -        if (ret) {
> > -            perror("os_mem_prealloc: failed to reinstall signal handler");
> > -            exit(1);
> > -        }
> > -
> > -        pthread_sigmask(SIG_SETMASK, &oldset, NULL);
> > +    ret = sigaction(SIGBUS, &oldact, NULL);
> > +    if (ret) {
> > +        perror("os_mem_prealloc: failed to reinstall signal handler");
> > +        exit(1);  
> 
> I guess you're keeping the exit() here, because you can't recover
> cleanly from this error.  I should never happen anyway.  Worth a
> comment, I think.
I didn't added comment because it's obvious that it's not possible
to recover and also because it's just the same old code that haven't
had a comment to begin with (probably also due to its obviousness)

> >      }
> > +    pthread_sigmask(SIG_SETMASK, &oldset, NULL);
> >  }
> >  
> >  
> > diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> > index 6debc2b..4c1dcf1 100644
> > --- a/util/oslib-win32.c
> > +++ b/util/oslib-win32.c
> > @@ -539,7 +539,7 @@ int getpagesize(void)
> >      return system_info.dwPageSize;
> >  }
> >  
> > -void os_mem_prealloc(int fd, char *area, size_t memory)
> > +void os_mem_prealloc(int fd, char *area, size_t memory, Error **errp)
> >  {
> >      int i;
> >      size_t pagesize = getpagesize();  
> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Thanks 

Re: [Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Paolo Bonzini]

> > On 21/07/2016 10:36, Markus Armbruster wrote:
> > > ... have the obvious error_propagate() + return here.  Matter of taste,
> > > between you and the maintainer.  Except there is none.  Inexcusable for
> > > a file created in 2014.  Suggest you appoint yourself.
> > 
> > For now I've queued the patch
> thanks
> 
> >, but I suggest that memory backends go
> > together with numa.c.
> Do you mean via Eduardo tree since here is numa maintainer?

Yes, for the future.

Paolo

Re: [Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Markus Armbruster]

Igor Mammedov <imammedo@redhat.com> writes:

> On Thu, 21 Jul 2016 10:36:40 +0200
> Markus Armbruster <armbru@redhat.com> wrote:
>
>> Igor Mammedov <imammedo@redhat.com> writes:
[...]
>> > @@ -352,15 +352,14 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
>> >          for (i = 0; i < numpages; i++) {
>> >              memset(area + (hpagesize * i), 0, 1);
>> >          }
>> > +    }
>> >  
>> > -        ret = sigaction(SIGBUS, &oldact, NULL);
>> > -        if (ret) {
>> > -            perror("os_mem_prealloc: failed to reinstall signal handler");
>> > -            exit(1);
>> > -        }
>> > -
>> > -        pthread_sigmask(SIG_SETMASK, &oldset, NULL);
>> > +    ret = sigaction(SIGBUS, &oldact, NULL);
>> > +    if (ret) {
>> > +        perror("os_mem_prealloc: failed to reinstall signal handler");
>> > +        exit(1);  
>> 
>> I guess you're keeping the exit() here, because you can't recover
>> cleanly from this error.  I should never happen anyway.  Worth a
>> comment, I think.
> I didn't added comment because it's obvious that it's not possible
> to recover and also because it's just the same old code that haven't
> had a comment to begin with (probably also due to its obviousness)

It's obvious enough once you read closely enough to see what the failing
code is doing.

The old code uses perror() + exit() consistently.  The new code mixes
Error with them, which is an anti-pattern.  That's okay, the exit() is
perfectly fine here.  The issue for me is that the anti-pattern triggers
the WTF-detector at a glance, but then I have to read closely to see #1
it's intentional, and #2 it's okay.  That wasn't the case before.

[...]

Re: [Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Igor Mammedov]

On Fri, 22 Jul 2016 18:19:44 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Igor Mammedov <imammedo@redhat.com> writes:
> 
> > On Thu, 21 Jul 2016 10:36:40 +0200
> > Markus Armbruster <armbru@redhat.com> wrote:
> >  
> >> Igor Mammedov <imammedo@redhat.com> writes:  
> [...]
> >> > @@ -352,15 +352,14 @@ void os_mem_prealloc(int fd, char *area, size_t memory)
> >> >          for (i = 0; i < numpages; i++) {
> >> >              memset(area + (hpagesize * i), 0, 1);
> >> >          }
> >> > +    }
> >> >  
> >> > -        ret = sigaction(SIGBUS, &oldact, NULL);
> >> > -        if (ret) {
> >> > -            perror("os_mem_prealloc: failed to reinstall signal handler");
> >> > -            exit(1);
> >> > -        }
> >> > -
> >> > -        pthread_sigmask(SIG_SETMASK, &oldset, NULL);
> >> > +    ret = sigaction(SIGBUS, &oldact, NULL);
> >> > +    if (ret) {
> >> > +        perror("os_mem_prealloc: failed to reinstall signal handler");
> >> > +        exit(1);    
> >> 
> >> I guess you're keeping the exit() here, because you can't recover
> >> cleanly from this error.  I should never happen anyway.  Worth a
> >> comment, I think.  
> > I didn't added comment because it's obvious that it's not possible
> > to recover and also because it's just the same old code that haven't
> > had a comment to begin with (probably also due to its obviousness)  
> 
> It's obvious enough once you read closely enough to see what the failing
> code is doing.
> 
> The old code uses perror() + exit() consistently.  The new code mixes
> Error with them, which is an anti-pattern.  That's okay, the exit() is
> perfectly fine here.  The issue for me is that the anti-pattern triggers
> the WTF-detector at a glance, but then I have to read closely to see #1
> it's intentional, and #2 it's okay.  That wasn't the case before.
> 
> [...]

I'll post a patch to add comment on top of this one.

[Qemu-devel] [PATCH] fixup! fix qemu exit on memory hotplug when allocation fails at prealloc time

[Igor Mammedov]

Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 - add comment o exit(1) path
---
 util/oslib-posix.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index a60ac97..f2d4e9e 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -356,6 +356,7 @@ void os_mem_prealloc(int fd, char *area, size_t memory, Error **errp)
 
     ret = sigaction(SIGBUS, &oldact, NULL);
     if (ret) {
+        /* Terminate QEMU since it can't recover from error */
         perror("os_mem_prealloc: failed to reinstall signal handler");
         exit(1);
     }
-- 
2.7.4

Re: [Qemu-devel] [PATCH] fixup! fix qemu exit on memory hotplug when allocation fails at prealloc time

[Markus Armbruster]

Igor Mammedov <imammedo@redhat.com> writes:

> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  - add comment o exit(1) path
> ---
>  util/oslib-posix.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index a60ac97..f2d4e9e 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -356,6 +356,7 @@ void os_mem_prealloc(int fd, char *area, size_t memory, Error **errp)
>  
>      ret = sigaction(SIGBUS, &oldact, NULL);
>      if (ret) {
> +        /* Terminate QEMU since it can't recover from error */
>          perror("os_mem_prealloc: failed to reinstall signal handler");
>          exit(1);
>      }

Thanks!  My R-by stands, of course.

Re: [Qemu-devel] [PATCH for-2.7] fix qemu exit on memory hotplug when allocation fails at prealloc time

[Eduardo Habkost]

On Thu, Jul 21, 2016 at 11:00:46AM -0400, Paolo Bonzini wrote:
> 
> > > On 21/07/2016 10:36, Markus Armbruster wrote:
> > > > ... have the obvious error_propagate() + return here.  Matter of taste,
> > > > between you and the maintainer.  Except there is none.  Inexcusable for
> > > > a file created in 2014.  Suggest you appoint yourself.
> > > 
> > > For now I've queued the patch
> > thanks
> > 
> > >, but I suggest that memory backends go
> > > together with numa.c.
> > Do you mean via Eduardo tree since here is numa maintainer?
> 
> Yes, for the future.

Works for me. I will submit a MAINTAINERS patch later, making
hostmem*.c part of the NUMA section.

-- 
Eduardo