* Re: [Qemu-devel] [PATCH v2] hostmem-file: reject invalid pmem file sizes
2019-02-13 7:14 [Qemu-devel] [PATCH v2] hostmem-file: reject invalid pmem file sizes Stefan Hajnoczi
@ 2019-02-13 8:13 ` Wei Yang
2019-02-14 2:46 ` Stefan Hajnoczi
2019-02-13 8:53 ` Wei Yang
2019-02-13 12:38 ` [Qemu-devel] [PATCH] hostmem: simplify ifdef-s in file_backend_memory_alloc() Igor Mammedov
2 siblings, 1 reply; 7+ messages in thread
From: Wei Yang @ 2019-02-13 8:13 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Haozhong Zhang, Eduardo Habkost, Stefan Weil,
Zhang Yi, Igor Mammedov
On Wed, Feb 13, 2019 at 03:14:01PM +0800, Stefan Hajnoczi wrote:
>Guests started with NVDIMMs larger than the underlying host file produce
>confusing errors inside the guest. This happens because the guest
>accesses pages beyond the end of the file.
>
>Check the pmem file size on startup and print a clear error message if
>the size is invalid.
>
>Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1669053
>Cc: Haozhong Zhang <haozhong.zhang@intel.com>
>Cc: Zhang Yi <yi.z.zhang@linux.intel.com>
>Cc: Eduardo Habkost <ehabkost@redhat.com>
>Cc: Igor Mammedov <imammedo@redhat.com>
>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>---
>v2:
> * Propagate qemu_get_pmem_size() errors [Igor]
>
> include/qemu/osdep.h | 13 ++++++++++
> backends/hostmem-file.c | 22 +++++++++++++++++
> util/oslib-posix.c | 53 +++++++++++++++++++++++++++++++++++++++++
> util/oslib-win32.c | 5 ++++
> 4 files changed, 93 insertions(+)
>
>diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>index 840af09cb0..303d315c5d 100644
>--- a/include/qemu/osdep.h
>+++ b/include/qemu/osdep.h
>@@ -570,6 +570,19 @@ void qemu_set_tty_echo(int fd, bool echo);
> void os_mem_prealloc(int fd, char *area, size_t sz, int smp_cpus,
> Error **errp);
>
>+/**
>+ * qemu_get_pmem_size:
>+ * @filename: path to a pmem file
>+ * @errp: pointer to a NULL-initialized error object
>+ *
>+ * Determine the size of a persistent memory file. Besides supporting files on
>+ * DAX file systems, this function also supports Linux devdax character
>+ * devices.
>+ *
>+ * Returns: the size or 0 on failure
>+
> /**
> * qemu_get_pid_name:
> * @pid: pid of a process
>diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
>index ba601ce940..d62689179b 100644
>--- a/backends/hostmem-file.c
>+++ b/backends/hostmem-file.c
>@@ -46,6 +46,28 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> gchar *name;
> #endif
>
>+ /*
>+ * Verify pmem file size since starting a guest with an incorrect size
>+ * leads to confusing failures inside the guest.
>+ */
>+ if (fb->is_pmem && fb->mem_path) {
>+ Error *local_err = NULL;
>+ uint64_t size;
>+
>+ size = qemu_get_pmem_size(fb->mem_path, &local_err);
>+ if (!size) {
>+ error_propagate(errp, local_err);
>+ return;
>+ }
>+
>+ if (backend->size > size) {
>+ error_setg(errp, "size property %" PRIu64 " is larger than "
>+ "pmem file \"%s\" size %" PRIu64, backend->size,
>+ fb->mem_path, size);
>+ return;
>+ }
>+ }
>+
> if (!backend->size) {
> error_setg(errp, "can't create backend with size 0");
> return;
Would it be reasonable to move the above check after checking size and
mem_path?
>diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>index 37c5854b9c..10d90d1783 100644
>--- a/util/oslib-posix.c
>+++ b/util/oslib-posix.c
>@@ -500,6 +500,59 @@ void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus,
> }
> }
>
>+uint64_t qemu_get_pmem_size(const char *filename, Error **errp)
>+{
>+ struct stat st;
>+
>+ if (stat(filename, &st) < 0) {
>+ error_setg(errp, "unable to stat pmem file \"%s\"", filename);
>+ return 0;
>+ }
>+
>+#if defined(__linux__)
>+ /* Special handling for devdax character devices */
>+ if (S_ISCHR(st.st_mode)) {
>+ char *subsystem_path = NULL;
>+ char *subsystem = NULL;
>+ char *size_path = NULL;
>+ char *size_str = NULL;
>+ uint64_t ret = 0;
>+
>+ subsystem_path = g_strdup_printf("/sys/dev/char/%d:%d/subsystem",
>+ major(st.st_rdev), minor(st.st_rdev));
>+ subsystem = g_file_read_link(subsystem_path, NULL);
>+ if (!subsystem) {
>+ error_setg(errp, "unable to read subsystem for pmem file \"%s\"",
>+ filename);
>+ goto devdax_err;
>+ }
>+
>+ if (!g_str_has_suffix(subsystem, "/dax")) {
>+ error_setg(errp, "pmem file \"%s\" is not a dax device", filename);
>+ goto devdax_err;
>+ }
>+
>+ size_path = g_strdup_printf("/sys/dev/char/%d:%d/size",
>+ major(st.st_rdev), minor(st.st_rdev));
>+ if (!g_file_get_contents(size_path, &size_str, NULL, NULL)) {
>+ error_setg(errp, "unable to read size for pmem file \"%s\"",
>+ size_path);
>+ goto devdax_err;
>+ }
>+
>+ ret = g_ascii_strtoull(size_str, NULL, 0);
>+
>+devdax_err:
>+ g_free(size_str);
>+ g_free(size_path);
>+ g_free(subsystem);
>+ g_free(subsystem_path);
>+ return ret;
>+ }
>+#endif /* defined(__linux__) */
>+
>+ return st.st_size;
>+}
>
> char *qemu_get_pid_name(pid_t pid)
> {
>diff --git a/util/oslib-win32.c b/util/oslib-win32.c
>index b4c17f5dfa..bd633afab6 100644
>--- a/util/oslib-win32.c
>+++ b/util/oslib-win32.c
>@@ -560,6 +560,11 @@ void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus,
> }
> }
>
>+uint64_t qemu_get_pmem_size(const char *filename, Error **errp)
>+{
>+ error_setg(errp, "pmem support not available");
>+ return 0;
>+}
>
> char *qemu_get_pid_name(pid_t pid)
> {
>--
>2.20.1
>
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH v2] hostmem-file: reject invalid pmem file sizes
2019-02-13 8:13 ` Wei Yang
@ 2019-02-14 2:46 ` Stefan Hajnoczi
0 siblings, 0 replies; 7+ messages in thread
From: Stefan Hajnoczi @ 2019-02-14 2:46 UTC (permalink / raw)
To: Wei Yang
Cc: qemu-devel, Haozhong Zhang, Eduardo Habkost, Stefan Weil,
Zhang Yi, Igor Mammedov
[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]
On Wed, Feb 13, 2019 at 04:13:18PM +0800, Wei Yang wrote:
> On Wed, Feb 13, 2019 at 03:14:01PM +0800, Stefan Hajnoczi wrote:
> >diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> >index ba601ce940..d62689179b 100644
> >--- a/backends/hostmem-file.c
> >+++ b/backends/hostmem-file.c
> >@@ -46,6 +46,28 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > gchar *name;
> > #endif
> >
> >+ /*
> >+ * Verify pmem file size since starting a guest with an incorrect size
> >+ * leads to confusing failures inside the guest.
> >+ */
> >+ if (fb->is_pmem && fb->mem_path) {
> >+ Error *local_err = NULL;
> >+ uint64_t size;
> >+
> >+ size = qemu_get_pmem_size(fb->mem_path, &local_err);
> >+ if (!size) {
> >+ error_propagate(errp, local_err);
> >+ return;
> >+ }
> >+
> >+ if (backend->size > size) {
> >+ error_setg(errp, "size property %" PRIu64 " is larger than "
> >+ "pmem file \"%s\" size %" PRIu64, backend->size,
> >+ fb->mem_path, size);
> >+ return;
> >+ }
> >+ }
> >+
> > if (!backend->size) {
> > error_setg(errp, "can't create backend with size 0");
> > return;
>
> Would it be reasonable to move the above check after checking size and
> mem_path?
Thanks, will fix in v3.
Stefan
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v2] hostmem-file: reject invalid pmem file sizes
2019-02-13 7:14 [Qemu-devel] [PATCH v2] hostmem-file: reject invalid pmem file sizes Stefan Hajnoczi
2019-02-13 8:13 ` Wei Yang
@ 2019-02-13 8:53 ` Wei Yang
2019-02-13 12:36 ` Igor Mammedov
2019-02-13 12:38 ` [Qemu-devel] [PATCH] hostmem: simplify ifdef-s in file_backend_memory_alloc() Igor Mammedov
2 siblings, 1 reply; 7+ messages in thread
From: Wei Yang @ 2019-02-13 8:53 UTC (permalink / raw)
To: Stefan Hajnoczi
Cc: qemu-devel, Haozhong Zhang, Eduardo Habkost, Stefan Weil,
Zhang Yi, Igor Mammedov
On Wed, Feb 13, 2019 at 03:14:01PM +0800, Stefan Hajnoczi wrote:
>Guests started with NVDIMMs larger than the underlying host file produce
>confusing errors inside the guest. This happens because the guest
>accesses pages beyond the end of the file.
>
>Check the pmem file size on startup and print a clear error message if
>the size is invalid.
>
>Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1669053
>Cc: Haozhong Zhang <haozhong.zhang@intel.com>
>Cc: Zhang Yi <yi.z.zhang@linux.intel.com>
>Cc: Eduardo Habkost <ehabkost@redhat.com>
>Cc: Igor Mammedov <imammedo@redhat.com>
>Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>---
>v2:
> * Propagate qemu_get_pmem_size() errors [Igor]
>
> include/qemu/osdep.h | 13 ++++++++++
> backends/hostmem-file.c | 22 +++++++++++++++++
> util/oslib-posix.c | 53 +++++++++++++++++++++++++++++++++++++++++
> util/oslib-win32.c | 5 ++++
> 4 files changed, 93 insertions(+)
>
>diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
>index 840af09cb0..303d315c5d 100644
>--- a/include/qemu/osdep.h
>+++ b/include/qemu/osdep.h
>@@ -570,6 +570,19 @@ void qemu_set_tty_echo(int fd, bool echo);
> void os_mem_prealloc(int fd, char *area, size_t sz, int smp_cpus,
> Error **errp);
>
>+/**
>+ * qemu_get_pmem_size:
>+ * @filename: path to a pmem file
>+ * @errp: pointer to a NULL-initialized error object
>+ *
>+ * Determine the size of a persistent memory file. Besides supporting files on
>+ * DAX file systems, this function also supports Linux devdax character
>+ * devices.
>+ *
>+ * Returns: the size or 0 on failure
>+ */
>+uint64_t qemu_get_pmem_size(const char *filename, Error **errp);
>+
> /**
> * qemu_get_pid_name:
> * @pid: pid of a process
>diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
>index ba601ce940..d62689179b 100644
>--- a/backends/hostmem-file.c
>+++ b/backends/hostmem-file.c
>@@ -46,6 +46,28 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> gchar *name;
> #endif
>
>+ /*
>+ * Verify pmem file size since starting a guest with an incorrect size
>+ * leads to confusing failures inside the guest.
>+ */
>+ if (fb->is_pmem && fb->mem_path) {
>+ Error *local_err = NULL;
>+ uint64_t size;
>+
>+ size = qemu_get_pmem_size(fb->mem_path, &local_err);
>+ if (!size) {
>+ error_propagate(errp, local_err);
>+ return;
>+ }
>+
>+ if (backend->size > size) {
>+ error_setg(errp, "size property %" PRIu64 " is larger than "
>+ "pmem file \"%s\" size %" PRIu64, backend->size,
>+ fb->mem_path, size);
>+ return;
>+ }
>+ }
>+
-mem-path is only supported on POSIX host.
So how about put this in the last code block before doing the real memory
region init?
BTW, I suggest to cleanup file_backend_memory_alloc() to make POSIX/non-POSIX
situation clear to understand.
> if (!backend->size) {
> error_setg(errp, "can't create backend with size 0");
> return;
>diff --git a/util/oslib-posix.c b/util/oslib-posix.c
>index 37c5854b9c..10d90d1783 100644
>--- a/util/oslib-posix.c
>+++ b/util/oslib-posix.c
>@@ -500,6 +500,59 @@ void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus,
> }
> }
>
>+uint64_t qemu_get_pmem_size(const char *filename, Error **errp)
>+{
>+ struct stat st;
>+
>+ if (stat(filename, &st) < 0) {
>+ error_setg(errp, "unable to stat pmem file \"%s\"", filename);
>+ return 0;
>+ }
>+
>+#if defined(__linux__)
>+ /* Special handling for devdax character devices */
>+ if (S_ISCHR(st.st_mode)) {
>+ char *subsystem_path = NULL;
>+ char *subsystem = NULL;
>+ char *size_path = NULL;
>+ char *size_str = NULL;
>+ uint64_t ret = 0;
>+
>+ subsystem_path = g_strdup_printf("/sys/dev/char/%d:%d/subsystem",
>+ major(st.st_rdev), minor(st.st_rdev));
>+ subsystem = g_file_read_link(subsystem_path, NULL);
>+ if (!subsystem) {
>+ error_setg(errp, "unable to read subsystem for pmem file \"%s\"",
>+ filename);
>+ goto devdax_err;
>+ }
>+
>+ if (!g_str_has_suffix(subsystem, "/dax")) {
>+ error_setg(errp, "pmem file \"%s\" is not a dax device", filename);
>+ goto devdax_err;
>+ }
>+
>+ size_path = g_strdup_printf("/sys/dev/char/%d:%d/size",
>+ major(st.st_rdev), minor(st.st_rdev));
>+ if (!g_file_get_contents(size_path, &size_str, NULL, NULL)) {
>+ error_setg(errp, "unable to read size for pmem file \"%s\"",
>+ size_path);
>+ goto devdax_err;
>+ }
>+
>+ ret = g_ascii_strtoull(size_str, NULL, 0);
>+
>+devdax_err:
>+ g_free(size_str);
>+ g_free(size_path);
>+ g_free(subsystem);
>+ g_free(subsystem_path);
>+ return ret;
>+ }
>+#endif /* defined(__linux__) */
>+
>+ return st.st_size;
>+}
>
> char *qemu_get_pid_name(pid_t pid)
> {
>diff --git a/util/oslib-win32.c b/util/oslib-win32.c
>index b4c17f5dfa..bd633afab6 100644
>--- a/util/oslib-win32.c
>+++ b/util/oslib-win32.c
>@@ -560,6 +560,11 @@ void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus,
> }
> }
>
>+uint64_t qemu_get_pmem_size(const char *filename, Error **errp)
>+{
>+ error_setg(errp, "pmem support not available");
>+ return 0;
>+}
>
> char *qemu_get_pid_name(pid_t pid)
> {
>--
>2.20.1
>
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH v2] hostmem-file: reject invalid pmem file sizes
2019-02-13 8:53 ` Wei Yang
@ 2019-02-13 12:36 ` Igor Mammedov
0 siblings, 0 replies; 7+ messages in thread
From: Igor Mammedov @ 2019-02-13 12:36 UTC (permalink / raw)
To: Wei Yang
Cc: Stefan Hajnoczi, Haozhong Zhang, Eduardo Habkost, Stefan Weil,
qemu-devel, Zhang Yi
On Wed, 13 Feb 2019 16:53:54 +0800
Wei Yang <richardw.yang@linux.intel.com> wrote:
> On Wed, Feb 13, 2019 at 03:14:01PM +0800, Stefan Hajnoczi wrote:
> >Guests started with NVDIMMs larger than the underlying host file produce
> >confusing errors inside the guest. This happens because the guest
> >accesses pages beyond the end of the file.
> >
> >Check the pmem file size on startup and print a clear error message if
> >the size is invalid.
> >
> >Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1669053
> >Cc: Haozhong Zhang <haozhong.zhang@intel.com>
> >Cc: Zhang Yi <yi.z.zhang@linux.intel.com>
> >Cc: Eduardo Habkost <ehabkost@redhat.com>
> >Cc: Igor Mammedov <imammedo@redhat.com>
> >Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> >---
> >v2:
> > * Propagate qemu_get_pmem_size() errors [Igor]
> >
> > include/qemu/osdep.h | 13 ++++++++++
> > backends/hostmem-file.c | 22 +++++++++++++++++
> > util/oslib-posix.c | 53 +++++++++++++++++++++++++++++++++++++++++
> > util/oslib-win32.c | 5 ++++
> > 4 files changed, 93 insertions(+)
> >
> >diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> >index 840af09cb0..303d315c5d 100644
> >--- a/include/qemu/osdep.h
> >+++ b/include/qemu/osdep.h
> >@@ -570,6 +570,19 @@ void qemu_set_tty_echo(int fd, bool echo);
> > void os_mem_prealloc(int fd, char *area, size_t sz, int smp_cpus,
> > Error **errp);
> >
> >+/**
> >+ * qemu_get_pmem_size:
> >+ * @filename: path to a pmem file
> >+ * @errp: pointer to a NULL-initialized error object
> >+ *
> >+ * Determine the size of a persistent memory file. Besides supporting files on
> >+ * DAX file systems, this function also supports Linux devdax character
> >+ * devices.
> >+ *
> >+ * Returns: the size or 0 on failure
> >+ */
> >+uint64_t qemu_get_pmem_size(const char *filename, Error **errp);
> >+
> > /**
> > * qemu_get_pid_name:
> > * @pid: pid of a process
> >diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
> >index ba601ce940..d62689179b 100644
> >--- a/backends/hostmem-file.c
> >+++ b/backends/hostmem-file.c
> >@@ -46,6 +46,28 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> > gchar *name;
> > #endif
> >
> >+ /*
> >+ * Verify pmem file size since starting a guest with an incorrect size
> >+ * leads to confusing failures inside the guest.
> >+ */
> >+ if (fb->is_pmem && fb->mem_path) {
> >+ Error *local_err = NULL;
> >+ uint64_t size;
> >+
> >+ size = qemu_get_pmem_size(fb->mem_path, &local_err);
> >+ if (!size) {
> >+ error_propagate(errp, local_err);
> >+ return;
> >+ }
> >+
> >+ if (backend->size > size) {
> >+ error_setg(errp, "size property %" PRIu64 " is larger than "
> >+ "pmem file \"%s\" size %" PRIu64, backend->size,
> >+ fb->mem_path, size);
> >+ return;
> >+ }
> >+ }
> >+
>
> -mem-path is only supported on POSIX host.
>
> So how about put this in the last code block before doing the real memory
> region init?
Indeed, evaluating file would be better done only if we didn't fail size/mempath checks first.
> BTW, I suggest to cleanup file_backend_memory_alloc() to make POSIX/non-POSIX
> situation clear to understand.
that's a separate patch, I'll reply with it here so Sefan could pick it (due to conflicts the change would create)
>
> > if (!backend->size) {
> > error_setg(errp, "can't create backend with size 0");
> > return;
> >diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> >index 37c5854b9c..10d90d1783 100644
> >--- a/util/oslib-posix.c
> >+++ b/util/oslib-posix.c
> >@@ -500,6 +500,59 @@ void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus,
> > }
> > }
> >
> >+uint64_t qemu_get_pmem_size(const char *filename, Error **errp)
> >+{
> >+ struct stat st;
> >+
> >+ if (stat(filename, &st) < 0) {
> >+ error_setg(errp, "unable to stat pmem file \"%s\"", filename);
> >+ return 0;
> >+ }
> >+
> >+#if defined(__linux__)
> >+ /* Special handling for devdax character devices */
> >+ if (S_ISCHR(st.st_mode)) {
> >+ char *subsystem_path = NULL;
> >+ char *subsystem = NULL;
> >+ char *size_path = NULL;
> >+ char *size_str = NULL;
> >+ uint64_t ret = 0;
> >+
> >+ subsystem_path = g_strdup_printf("/sys/dev/char/%d:%d/subsystem",
> >+ major(st.st_rdev), minor(st.st_rdev));
> >+ subsystem = g_file_read_link(subsystem_path, NULL);
> >+ if (!subsystem) {
> >+ error_setg(errp, "unable to read subsystem for pmem file \"%s\"",
> >+ filename);
> >+ goto devdax_err;
> >+ }
> >+
> >+ if (!g_str_has_suffix(subsystem, "/dax")) {
> >+ error_setg(errp, "pmem file \"%s\" is not a dax device", filename);
> >+ goto devdax_err;
> >+ }
> >+
> >+ size_path = g_strdup_printf("/sys/dev/char/%d:%d/size",
> >+ major(st.st_rdev), minor(st.st_rdev));
> >+ if (!g_file_get_contents(size_path, &size_str, NULL, NULL)) {
> >+ error_setg(errp, "unable to read size for pmem file \"%s\"",
> >+ size_path);
> >+ goto devdax_err;
> >+ }
> >+
> >+ ret = g_ascii_strtoull(size_str, NULL, 0);
> >+
> >+devdax_err:
> >+ g_free(size_str);
> >+ g_free(size_path);
> >+ g_free(subsystem);
> >+ g_free(subsystem_path);
> >+ return ret;
> >+ }
> >+#endif /* defined(__linux__) */
> >+
> >+ return st.st_size;
> >+}
> >
> > char *qemu_get_pid_name(pid_t pid)
> > {
> >diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> >index b4c17f5dfa..bd633afab6 100644
> >--- a/util/oslib-win32.c
> >+++ b/util/oslib-win32.c
> >@@ -560,6 +560,11 @@ void os_mem_prealloc(int fd, char *area, size_t memory, int smp_cpus,
> > }
> > }
> >
> >+uint64_t qemu_get_pmem_size(const char *filename, Error **errp)
> >+{
> >+ error_setg(errp, "pmem support not available");
> >+ return 0;
> >+}
> >
> > char *qemu_get_pid_name(pid_t pid)
> > {
> >--
> >2.20.1
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH] hostmem: simplify ifdef-s in file_backend_memory_alloc()
2019-02-13 7:14 [Qemu-devel] [PATCH v2] hostmem-file: reject invalid pmem file sizes Stefan Hajnoczi
2019-02-13 8:13 ` Wei Yang
2019-02-13 8:53 ` Wei Yang
@ 2019-02-13 12:38 ` Igor Mammedov
2019-02-13 13:34 ` Wei Yang
2 siblings, 1 reply; 7+ messages in thread
From: Igor Mammedov @ 2019-02-13 12:38 UTC (permalink / raw)
To: qemu-devel
Cc: Haozhong Zhang, Zhang Yi, Eduardo Habkost, Stefan Hajnoczi,
Wei Yang
cleanup file_backend_memory_alloc() by using one CONFIG_POSIX ifdef
instead of several ones within the function to make it simpler to follow.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
Suggested-by: Wei Yang <richardw.yang@linux.intel.com>
---
Cc: Haozhong Zhang <haozhong.zhang@intel.com>
Cc: Zhang Yi <yi.z.zhang@linux.intel.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
CC: Wei Yang <richardw.yang@linux.intel.com>
---
backends/hostmem-file.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
index ba601ce940..ce54788048 100644
--- a/backends/hostmem-file.c
+++ b/backends/hostmem-file.c
@@ -41,10 +41,12 @@ struct HostMemoryBackendFile {
static void
file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
{
+#ifndef CONFIG_POSIX
+ error_setg(errp, "backend '%s' not supported on this host",
+ object_get_typename(OBJECT(backend)));
+#else
HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
-#ifdef CONFIG_POSIX
gchar *name;
-#endif
if (!backend->size) {
error_setg(errp, "can't create backend with size 0");
@@ -54,9 +56,6 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
error_setg(errp, "mem-path property not set");
return;
}
-#ifndef CONFIG_POSIX
- error_setg(errp, "-mem-path not supported on this host");
-#else
backend->force_prealloc = mem_prealloc;
name = host_memory_backend_get_name(backend);
memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
--
2.18.1
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [Qemu-devel] [PATCH] hostmem: simplify ifdef-s in file_backend_memory_alloc()
2019-02-13 12:38 ` [Qemu-devel] [PATCH] hostmem: simplify ifdef-s in file_backend_memory_alloc() Igor Mammedov
@ 2019-02-13 13:34 ` Wei Yang
0 siblings, 0 replies; 7+ messages in thread
From: Wei Yang @ 2019-02-13 13:34 UTC (permalink / raw)
To: Igor Mammedov
Cc: qemu-devel, Haozhong Zhang, Wei Yang, Eduardo Habkost,
Stefan Hajnoczi, Zhang Yi
On Wed, Feb 13, 2019 at 07:38:58AM -0500, Igor Mammedov wrote:
>cleanup file_backend_memory_alloc() by using one CONFIG_POSIX ifdef
>instead of several ones within the function to make it simpler to follow.
>
>Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>Suggested-by: Wei Yang <richardw.yang@linux.intel.com>
Reviewed-by: Wei Yang <richardw.yang@linux.intel.com>
>---
>Cc: Haozhong Zhang <haozhong.zhang@intel.com>
>Cc: Zhang Yi <yi.z.zhang@linux.intel.com>
>Cc: Eduardo Habkost <ehabkost@redhat.com>
>Cc: Stefan Hajnoczi <stefanha@redhat.com>
>CC: Wei Yang <richardw.yang@linux.intel.com>
>---
> backends/hostmem-file.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
>diff --git a/backends/hostmem-file.c b/backends/hostmem-file.c
>index ba601ce940..ce54788048 100644
>--- a/backends/hostmem-file.c
>+++ b/backends/hostmem-file.c
>@@ -41,10 +41,12 @@ struct HostMemoryBackendFile {
> static void
> file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> {
>+#ifndef CONFIG_POSIX
>+ error_setg(errp, "backend '%s' not supported on this host",
>+ object_get_typename(OBJECT(backend)));
>+#else
> HostMemoryBackendFile *fb = MEMORY_BACKEND_FILE(backend);
>-#ifdef CONFIG_POSIX
> gchar *name;
>-#endif
>
> if (!backend->size) {
> error_setg(errp, "can't create backend with size 0");
>@@ -54,9 +56,6 @@ file_backend_memory_alloc(HostMemoryBackend *backend, Error **errp)
> error_setg(errp, "mem-path property not set");
> return;
> }
>-#ifndef CONFIG_POSIX
>- error_setg(errp, "-mem-path not supported on this host");
>-#else
> backend->force_prealloc = mem_prealloc;
> name = host_memory_backend_get_name(backend);
> memory_region_init_ram_from_file(&backend->mr, OBJECT(backend),
>--
>2.18.1
>
--
Wei Yang
Help you, Help me
^ permalink raw reply [flat|nested] 7+ messages in thread