Re: [Qemu-devel] [PATCH] xen: don't save/restore the physmap on VM save/restore
Hi, This series failed automatic build test. Please find the testing commands and their output below. If you have docker installed, you can probably reproduce it locally. Type: series Subject: [Qemu-devel] [PATCH] xen: don't save/restore the physmap on VM save/restore Message-id: 1489084689-19008-1-git-send-email-igor.druzhi...@citrix.com === TEST SCRIPT BEGIN === #!/bin/bash set -e git submodule update --init dtc # Let docker tests dump environment info export SHOW_ENV=1 export J=16 make docker-test-quick@centos6 make docker-test-mingw@fedora make docker-test-build@min-glib === TEST SCRIPT END === Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 Switched to a new branch 'test' ebd2fd8 xen: don't save/restore the physmap on VM save/restore === OUTPUT BEGIN === Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc' Cloning into 'dtc'... Submodule path 'dtc': checked out 'fa8bc7f928ac25f23532afc8beb2073efc8fb063' BUILD centos6 make[1]: Entering directory `/var/tmp/patchew-tester-tmp-o3dzprlk/src' ARCHIVE qemu.tgz ARCHIVE dtc.tgz COPYRUNNER RUN test-quick in qemu:centos6 Packages installed: SDL-devel-1.2.14-7.el6_7.1.x86_64 ccache-3.1.6-2.el6.x86_64 epel-release-6-8.noarch gcc-4.4.7-17.el6.x86_64 git-1.7.1-4.el6_7.1.x86_64 glib2-devel-2.28.8-5.el6.x86_64 libfdt-devel-1.4.0-1.el6.x86_64 make-3.81-23.el6.x86_64 package g++ is not installed pixman-devel-0.32.8-1.el6.x86_64 tar-1.23-15.el6_8.x86_64 zlib-devel-1.2.3-29.el6.x86_64 Environment variables: PACKAGES=libfdt-devel ccache tar git make gcc g++ zlib-devel glib2-devel SDL-devel pixman-devel epel-release HOSTNAME=ac1377aa9f68 TERM=xterm MAKEFLAGS= -j16 HISTSIZE=1000 J=16 USER=root CCACHE_DIR=/var/tmp/ccache EXTRA_CONFIGURE_OPTS= V= SHOW_ENV=1 MAIL=/var/spool/mail/root PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin PWD=/ LANG=en_US.UTF-8 TARGET_LIST= HISTCONTROL=ignoredups SHLVL=1 HOME=/root TEST_DIR=/tmp/qemu-test LOGNAME=root LESSOPEN=||/usr/bin/lesspipe.sh %s FEATURES= dtc DEBUG= G_BROKEN_FILENAMES=1 CCACHE_HASHDIR= _=/usr/bin/env Configure options: --enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/var/tmp/qemu-build/install No C++ compiler available; disabling C++ specific optional code Install prefix/var/tmp/qemu-build/install BIOS directory/var/tmp/qemu-build/install/share/qemu binary directory /var/tmp/qemu-build/install/bin library directory /var/tmp/qemu-build/install/lib module directory /var/tmp/qemu-build/install/lib/qemu libexec directory /var/tmp/qemu-build/install/libexec include directory /var/tmp/qemu-build/install/include config directory /var/tmp/qemu-build/install/etc local state directory /var/tmp/qemu-build/install/var Manual directory /var/tmp/qemu-build/install/share/man ELF interp prefix /usr/gnemul/qemu-%M Source path /tmp/qemu-test/src C compilercc Host C compiler cc C++ compiler Objective-C compiler cc ARFLAGS rv CFLAGS-O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g QEMU_CFLAGS -I/usr/include/pixman-1 -I$(SRC_PATH)/dtc/libfdt -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -fPIE -DPIE -m64 -mcx16 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv -Wendif-labels -Wno-missing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all LDFLAGS -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g make make install install pythonpython -B smbd /usr/sbin/smbd module supportno host CPU x86_64 host big endian no target list x86_64-softmmu aarch64-softmmu tcg debug enabled no gprof enabled no sparse enabledno strip binariesyes profiler no static build no pixmansystem SDL support yes (1.2.14) GTK support no GTK GL supportno VTE support no TLS priority NORMAL GNUTLS supportno GNUTLS rndno libgcrypt no libgcrypt kdf no nettleno nettle kdfno libtasn1 no curses supportno virgl support no curl support no mingw32 support no Audio drivers oss Block whitelist (rw) Block whitelist (ro) VirtFS supportno VNC support yes VNC SASL support no VNC JPEG support no VNC PNG support no xen support no brlapi supportno bluez supportno Documentation no PIE yes vde support no netmap supportno Linux AIO support no ATTR/XATTR support yes Install blobs yes KVM support yes HAX support no RDMA support no
Re: [Qemu-devel] [PATCH] xen: don't save/restore the physmap on VM save/restore
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PATCH] xen: don't save/restore the physmap on VM
save/restore
Message-id: 1489084689-19008-1-git-send-email-igor.druzhi...@citrix.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
* [new tag]
patchew/1489084689-19008-1-git-send-email-igor.druzhi...@citrix.com ->
patchew/1489084689-19008-1-git-send-email-igor.druzhi...@citrix.com
Switched to a new branch 'test'
ebd2fd8 xen: don't save/restore the physmap on VM save/restore
=== OUTPUT BEGIN ===
Checking PATCH 1/1: xen: don't save/restore the physmap on VM save/restore...
ERROR: space prohibited between function name and open parenthesis '('
#105: FILE: xen-hvm.c:344:
+physmap = g_malloc(sizeof (XenPhysmap));
ERROR: that open brace { should be on the previous line
#114: FILE: xen-hvm.c:353:
+if (runstate_check(RUN_STATE_INMIGRATE))
+{
ERROR: trailing whitespace
#117: FILE: xen-hvm.c:356:
+ * established during the restore phase so we can safely update the $
ERROR: braces {} are necessary for all arms of this statement
#119: FILE: xen-hvm.c:358:
+ if (mr == framebuffer)
[...]
total: 4 errors, 0 warnings, 196 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@freelists.org
[Qemu-devel] [PATCH] xen: don't save/restore the physmap on VM save/restore
Saving/restoring the physmap to/from xenstore was introduced to
QEMU majorly in order to cover up the VRAM region restore issue.
The sequence of restore operations implies that we should know
the effective guest VRAM address *before* we have the VRAM region
restored (which happens later). Unfortunately, in Xen environment
VRAM memory does actually belong to a guest - not QEMU itself -
which means the position of this region is unknown beforehand and
can't be mapped into QEMU address space immediately.
Previously, recreating xenstore keys, holding the physmap, by the
toolstack helped to get this information in place at the right
moment ready to be consumed by QEMU to map the region properly.
The extraneous complexity of having those keys transferred by the
toolstack and unnecessary redundancy prompted us to propose a
solution which doesn't require any extra data in xenstore. The idea
is to defer the VRAM region mapping till the point we actually know
the effective address and able to map it. To that end, we initially
only register the pointer to the framebuffer without actual mapping.
Then, during the memory region restore phase, we perform the mapping
of the known address and update the VRAM region metadata (including
previously registered pointer) accordingly.
Signed-off-by: Igor Druzhinin
---
exec.c | 3 ++
hw/display/vga.c | 2 +-
include/hw/xen/xen.h | 2 +-
xen-hvm.c| 114 ---
4 files changed, 32 insertions(+), 89 deletions(-)
diff --git a/exec.c b/exec.c
index aabb035..5f2809e 100644
--- a/exec.c
+++ b/exec.c
@@ -2008,6 +2008,9 @@ void *qemu_map_ram_ptr(RAMBlock *ram_block, ram_addr_t
addr)
}
block->host = xen_map_cache(block->offset, block->max_length, 1);
+if (block->host == NULL) {
+return NULL;
+}
}
return ramblock_ptr(block, addr);
}
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 69c3e1d..be554c2 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -2163,7 +2163,7 @@ void vga_common_init(VGACommonState *s, Object *obj, bool
global_vmstate)
memory_region_init_ram(&s->vram, obj, "vga.vram", s->vram_size,
&error_fatal);
vmstate_register_ram(&s->vram, global_vmstate ? NULL : DEVICE(obj));
-xen_register_framebuffer(&s->vram);
+xen_register_framebuffer(&s->vram, &s->vram_ptr);
s->vram_ptr = memory_region_get_ram_ptr(&s->vram);
s->get_bpp = vga_get_bpp;
s->get_offsets = vga_get_offsets;
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index 09c2ce5..3831843 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -45,6 +45,6 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
struct MemoryRegion *mr, Error **errp);
void xen_modified_memory(ram_addr_t start, ram_addr_t length);
-void xen_register_framebuffer(struct MemoryRegion *mr);
+void xen_register_framebuffer(struct MemoryRegion *mr, uint8_t **ptr);
#endif /* QEMU_HW_XEN_H */
diff --git a/xen-hvm.c b/xen-hvm.c
index 5043beb..ea5ed24 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -41,6 +41,7 @@
static MemoryRegion ram_memory, ram_640k, ram_lo, ram_hi;
static MemoryRegion *framebuffer;
+static uint8_t **framebuffer_ptr;
static bool xen_in_migration;
/* Compatibility with older version */
@@ -302,7 +303,6 @@ static hwaddr xen_phys_offset_to_gaddr(hwaddr start_addr,
return physmap->start_addr;
}
}
-
return start_addr;
}
@@ -317,7 +317,6 @@ static int xen_add_to_physmap(XenIOState *state,
XenPhysmap *physmap = NULL;
hwaddr pfn, start_gpfn;
hwaddr phys_offset = memory_region_get_ram_addr(mr);
-char path[80], value[17];
const char *mr_name;
if (get_physmapping(state, start_addr, size)) {
@@ -340,6 +339,27 @@ go_physmap:
DPRINTF("mapping vram to %"HWADDR_PRIx" - %"HWADDR_PRIx"\n",
start_addr, start_addr + size);
+mr_name = memory_region_name(mr);
+
+physmap = g_malloc(sizeof (XenPhysmap));
+
+physmap->start_addr = start_addr;
+physmap->size = size;
+physmap->name = mr_name;
+physmap->phys_offset = phys_offset;
+
+QLIST_INSERT_HEAD(&state->physmap, physmap, list);
+
+if (runstate_check(RUN_STATE_INMIGRATE))
+{
+/* At this point we have a physmap entry for the framebuffer region
+ * established during the restore phase so we can safely update the
+ * registered framebuffer address here. */
+ if (mr == framebuffer)
+*framebuffer_ptr = memory_region_get_ram_ptr(framebuffer);
+return 0;
+}
+
pfn = phys_offset >> TARGET_PAGE_BITS;
start_gpfn = start_addr >> TARGET_PAGE_BITS;
for (i = 0; i < size >> TARGET_PAGE_BITS; i++) {
@@ -350,49 +370,17 @@ go_physmap:
if (rc) {
DPRINTF("add_to_physmap MFN %"PRI_xen_pfn" to PFN %"
PRI_xen_pfn" failed: %d (errno: %d)\n", id
