Re: [libvirt] [PATCH] nodeinfo: avoid uninitialized variable on error
On 11.06.2014 00:27, Eric Blake wrote: Commit 8ba0a58 introduced a compiler warning that I hit during a run of ./autobuild.sh: ../../src/nodeinfo.c: In function 'nodeCapsInitNUMA': ../../src/nodeinfo.c:1853:43: error: 'nsiblings' may be used uninitialized in this function [-Werror=maybe-uninitialized] if (virCapabilitiesAddHostNUMACell(caps, n, memory, ^ Sure enough, nsiblings starts uninitialized, and is set by a call to virNodeCapsGetSiblingInfo, but that function fails to assign through the pointer if virNumaGetDistances fails. If that's the case, virNodeCapsGetSiblingInfo() fails too so at the incriminated lines we jump at cleanup label directly. But maybe the compiler you've used fails to do analysis this deep. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 2/2] maint: prohibit empty first lines
On Tue, Jun 10, 2014 at 01:21:49PM -0600, Eric Blake wrote:
On 06/10/2014 12:18 PM, Roman Bogorodskiy wrote:
+sc_prohibit_empty_first_line:
+ @awk 'BEGIN { fail=0; } \
+ FNR == 1 { if ($$0 == ) { print FILENAME :1:; fail=1; } } \
+ END { if (fail == 1) { \
+ print $(ME): Prohibited empty first line /dev/stderr;\
+ } exit fail; }' $$($(VC_LIST_EXCEPT));
+
# We don't use this feature of maint.mk.
prev_version_file = /dev/null
@@ -1115,3 +1122,6 @@
exclude_file_name_regexp--sc_avoid_attribute_unused_in_header = \
exclude_file_name_regexp--sc_prohibit_mixed_case_abbreviations = \
^src/(vbox/vbox_CAPI.*.h|esx/esx_vi.(c|h)|esx/esx_storage_backend_iscsi.c)$$
+
+exclude_file_name_regexp--sc_prohibit_empty_first_line = \
+
^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/vmwareverdata/fusion-5.0.3.txt)$$
It started to fail for me with this change:
prohibit_empty_first_line
tools/libvirt_win_icon_16x16.ico:1:
Hmm, it might be simpler just to globally exclude all image formats,
rather than repeating lists of image files in affected rules.
Definitely. It's weird, though, that I'm not seeing this problem. Is
it because I'm building on Linux? Why didn't some test box we have
fail as well?
I was able to fix it with:
exclude_file_name_regexp--sc_prohibit_empty_first_line = \
-
^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/vmwareverdata/fusion-5.0.3.txt)$$
+
^(README|daemon/THREADS\.txt|src/esx/README|docs/library.xen|tests/vmwareverdata/fusion-5.0.3.txt|tools/.*\.ico)$$
Not quite consistent with other places where we ignore image files; for
example:
exclude_file_name_regexp--sc_trailing_blank = \
(/qemuhelpdata/|\.(fig|gif|ico|png)$$)
but my thoughts are that we should really ditch those one-off exception
lists for graphic binary files, and instead modify this list:
VC_LIST_ALWAYS_EXCLUDE_REGEX = \
(^(HACKING|docs/(news\.html\.in|.*\.patch))|\.po)$$
And this looks like the right way to go. It would deal with possible
future problems as well. After figuring out the parentheses, I can
ACK this to go in if you want. I'm hoping to get to cleaning up the
excludes one day.
Martin
signature.asc
Description: Digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Expose link state speed
On 06/10/2014 08:19 PM, Michal Privoznik wrote: On 05.06.2014 17:39, Michal Privoznik wrote: While the 1/4 was ACKed, I'm sending int for completeness. And I've made a tiny change: the link speed is no longer unsigned long but unsigned int instead. That'll do too and it consumes less memory. Michal Privoznik (4): virInterface: Expose link state speed virnetdev: Introduce virNetDevGetLinkInfo interface_backend_udev: Implement link speed state node_device: Expose link state speed FYI, I'm now working on adding this to netcf. I'll Cc you when I post the patch. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Expose link state speed
On 11.06.2014 10:02, Laine Stump wrote: On 06/10/2014 08:19 PM, Michal Privoznik wrote: On 05.06.2014 17:39, Michal Privoznik wrote: While the 1/4 was ACKed, I'm sending int for completeness. And I've made a tiny change: the link speed is no longer unsigned long but unsigned int instead. That'll do too and it consumes less memory. Michal Privoznik (4): virInterface: Expose link state speed virnetdev: Introduce virNetDevGetLinkInfo interface_backend_udev: Implement link speed state node_device: Expose link state speed FYI, I'm now working on adding this to netcf. I'll Cc you when I post the patch. Awesome. Is the XML schema I'm proposing here okay with the netcf or do we need I need to change something? If so, it's still time to change it - up till the release. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: exempt graphic binaries from syntax check
On Tue, Jun 10, 2014 at 01:39:03PM -0600, Eric Blake wrote: Roman Bogorodskiy reported a syntax-check failure when using FreeBSD; complaining that: prohibit_empty_first_line tools/libvirt_win_icon_16x16.ico:1: tools/libvirt_win_icon_32x32.ico:1: tools/libvirt_win_icon_48x48.ico:1: tools/libvirt_win_icon_64x64.ico:1: maint.mk: Prohibited empty first line In reality, the first 'line' of that file is NOT empty; but since it is a binary file, awk is not required to handle it gracefully. The simplest solution is to exempt all image files from syntax checks in the first place - after all, we only store them in git because they are inconvenient to regenerate, but they are not our preferred format for making modifications, and syntax check should only cover files that we are likely to modify. * cfg.mk (VC_LIST_ALWAYS_EXCLUDE_REGEX): Exempt images. (exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF): Simplify. (exclude_file_name_regexp--sc_trailing_blank): Likewise. ACK Signed-off-by: Eric Blake ebl...@redhat.com --- cfg.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cfg.mk b/cfg.mk index 10ad744..1f07639 100644 --- a/cfg.mk +++ b/cfg.mk @@ -90,7 +90,7 @@ endif # Files that should never cause syntax check failures. VC_LIST_ALWAYS_EXCLUDE_REGEX = \ - (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.po)$$ + (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.(po|fig|gif|ico|png))$$ # Functions like free() that are no-ops on NULL arguments. useless_free_options = \ @@ -1047,7 +1047,7 @@ exclude_file_name_regexp--sc_prohibit_close = \ (\.p[yl]$$|^docs/|^(src/util/virfile\.c|src/libvirt\.c|tests/vir(cgroup|pci)mock\.c)$$) exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF = \ - (^tests/(qemuhelp|nodeinfo|virpcitest)data/|\.(gif|ico|png|diff)$$) + (^tests/(qemuhelp|nodeinfo|virpcitest)data/|\.diff$$) _src2=src/(util/vircommand|libvirt|lxc/lxc_controller|locking/lock_daemon) exclude_file_name_regexp--sc_prohibit_fork_wrappers = \ @@ -1093,7 +1093,7 @@ exclude_file_name_regexp--sc_require_config_h_first = \ ^(examples/|tools/virsh-edit\.c$$) exclude_file_name_regexp--sc_trailing_blank = \ - (/qemuhelpdata/|/sysinfodata/.*\.data|\.(fig|gif|ico|png)$$) + /qemuhelpdata/|/sysinfodata/.*\.data$$ exclude_file_name_regexp--sc_unmarked_diagnostics = \ ^(docs/apibuild.py|tests/virt-aa-helper-test)$$ -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] build: remove ssp-buffer-size
This option only makes sense for -fstack-protector. With -fstack-protector-all or -fstack-protector-strong, functions are protected regardless of buffer size. https://bugzilla.redhat.com/show_bug.cgi?id=1105456 --- m4/virt-compile-warnings.m4 | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index fb82238..196afa7 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -166,16 +166,11 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ *-*-linux*) dnl Fedora only uses -fstack-protector, but doesn't seem to dnl be great overhead in adding -fstack-protector-all instead - dnl wantwarn=$wantwarn -fstack-protector + dnl + dnl We also don't need ssp-buffer-size with -all, + dnl since functions are protected regardless of buffer size. + dnl wantwarn=$wantwarn --param=ssp-buffer-size=4 wantwarn=$wantwarn -fstack-protector-all - wantwarn=$wantwarn --param=ssp-buffer-size=4 - dnl Even though it supports it, clang complains about - dnl use of --param=ssp-buffer-size=4 unless used with - dnl the -c arg. It doesn't like it when used with args - dnl that just link together .o files. Unfortunately - dnl we can't avoid that with automake, so we must turn - dnl off the following clang specific warning - wantwarn=$wantwarn -Wno-unused-command-line-argument ;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] build: use -fstack-protector-strong
Tested with gcc-4.9.0 and clang-3.4.1. Ján Tomko (3): build: remove duplicit warning suppression build: remove ssp-buffer-size build: prefer -fstack-protector-strong to -all m4/virt-compile-warnings.m4 | 36 +--- 1 file changed, 21 insertions(+), 15 deletions(-) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] build: prefer -fstack-protector-strong to -all
Check upfront if it's supported, to avoid putting both of them on the command line. --- m4/virt-compile-warnings.m4 | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 196afa7..6d632f9 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -156,6 +156,15 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ wantwarn=$wantwarn -Wframe-larger-than=4096 dnl wantwarn=$wantwarn -Wframe-larger-than=256 +AC_CACHE_CHECK([whether the C compiler supports stack-protector-strong], + [lv_cv_gcc_fstack_protector_strong], [ + save_CFLAGS=$CFLAGS + CFLAGS='-fstack-protector-strong -Werror' + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]])], + [lv_cv_gcc_fstack_protector_strong=yes], + [lv_cv_gcc_fstack_protector_strong=no]) + CFLAGS=$save_CFLAGS]) + # Extra special flags dnl -fstack-protector stuff passes gl_WARN_ADD with gcc dnl on Mingw32, but fails when actually used @@ -164,13 +173,18 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl error: -fstack-protector not supported for this target [-Werror] ;; *-*-linux*) - dnl Fedora only uses -fstack-protector, but doesn't seem to - dnl be great overhead in adding -fstack-protector-all instead + dnl Prefer -fstack-protector-strong if it's available. + dnl There doesn't seem to be great overhead in adding + dnl -fstack-protector-all instead of -fstack-protector. dnl - dnl We also don't need ssp-buffer-size with -all, + dnl We also don't need ssp-buffer-size with -all or -strong, dnl since functions are protected regardless of buffer size. dnl wantwarn=$wantwarn --param=ssp-buffer-size=4 - wantwarn=$wantwarn -fstack-protector-all + if test $lv_cv_gcc_fstack_protector_strong = yes; then + wantwarn=$wantwarn -fstack-protector-strong + else + wantwarn=$wantwarn -fstack-protector-all + fi ;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] build: remove duplicit warning suppression
These warnings have alread been added to $dontwarn. --- m4/virt-compile-warnings.m4 | 3 --- 1 file changed, 3 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 574fbc4..fb82238 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -119,9 +119,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # ideally we'd turn many of them on dontwarn=$dontwarn -Wfloat-equal dontwarn=$dontwarn -Wdeclaration-after-statement -dontwarn=$dontwarn -Wcast-qual -dontwarn=$dontwarn -Wconversion -dontwarn=$dontwarn -Wsign-conversion dontwarn=$dontwarn -Wpacked dontwarn=$dontwarn -Wunused-macros dontwarn=$dontwarn -Woverlength-strings -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] virnetdev: Introduce virNetDevGetLinkInfo
On 11.06.2014 03:13, John Ferlan wrote:
On 06/05/2014 11:39 AM, Michal Privoznik wrote:
The purpose of this function is to fetch link state
and link speed for given NIC name from the SYSFS.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/libvirt_private.syms | 1 +
src/util/virnetdev.c | 104 +++
src/util/virnetdev.h | 6 +++
3 files changed, 111 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 394c086..6d08ee9 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1540,6 +1540,7 @@ virNetDevClearIPv4Address;
virNetDevExists;
virNetDevGetIndex;
virNetDevGetIPv4Address;
+virNetDevGetLinkInfo;
virNetDevGetMAC;
virNetDevGetMTU;
virNetDevGetPhysicalFunction;
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 3a60cf7..9d2c0d2 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1832,3 +1832,107 @@ virNetDevRestoreNetConfig(const char *linkdev
ATTRIBUTE_UNUSED,
}
#endif /* defined(__linux__) defined(HAVE_LIBNL) */
+
+#ifdef __linux__
+# define SYSFS_PREFIX /sys/class/net/
There is a 'NET_SYSFS' definition earlier to the same path - seems it
could/should be reused...
NIT: Your definition will create a double slash below for operstate and
speed... Whether that's desired or not is up to you... There's a mix of
usage within libvirt sources.
In fact, I've substituted both virAsprintf()s with virNetDevSysfsFile()
which does exactly what I need.
+int
+virNetDevGetLinkInfo(const char *ifname,
+ virInterfaceState *state,
+ unsigned int *speed)
Could there ever be any other stats/data to fetch? Perhaps consider
passing a virInterfaceLinkPtr instead and then reference state/speed off
of it. Then in the future you don't have to keep adding params to the
API. None of the current callers pass NULL for either parameter.
Right. The API design is a leftover from previous patch set where I was
implementing netcf backend too. Fixed.
I also wonder if we really want to fail out if we something goes wrong.
It's not like we're affecting the state/speed of the device if something
did go wrong - we're just outputting data if it's there. I guess I'm
thinking more about the callers, but that's a design decision you have
to make and live with. Considering the comment below regarding the bug
with speed - one wonders what else lurks in former formats that the
following code can error out on.
I'd say error out in this function and let caller decide whether he
wants to ignore error or not. If I make this function fault tolerant,
caller loses that opportunity.
+{
+int ret = -1;
+char *path = NULL;
+char *buf = NULL;
+char *tmp;
+int tmp_state;
s/int/virInterfaceState/
In fact this is intentional. Remember Eric's TypeFromString() patches?
The problem is, one can't be sure if compiler decides on using unsigned
or signed integer to represent an enum. If it decides to use an unsigned
int (IIRC that's the case of older gcc) then comparison a few lines
below will never be true. That's why I'm introducing this dummy
variable. I could as well assign the result to the destination variable
directly. BTW: old compilers are the reason why I'm crippling some
variable names, e.g. 'virInterfaceLinkPtr lnk' vs. 'virInterfaceLinkPtr
link' as older gcc fails to see 'link' is a variable not the 'link()'
function.
Seems we should be able to set up a dummy /sys/class/net environment in
order to test the new API's... Something I attempted with my recently
posted scsi_host changes.
Yeah, unit testing is certainly in place. But honestly, I thought it
would blow the size of this patches up. Again, something that a follow
up patch will do.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] virInterface: Expose link state speed
On 11.06.2014 03:13, John Ferlan wrote: On 06/05/2014 11:39 AM, Michal Privoznik wrote: Currently it is not possible to determine the speed of an interface and whether a link is actually detected from the API. Orchestrating platforms want to be able to determine when the link has failed and where multiple speeds may be available which one the interface is actually connected at. This commit introduces an extension to our interface XML (without implementation to interface driver backends): interface type='ethernet' name='eth0' start mode='none'/ mac address='aa:bb:cc:dd:ee:ff'/ link speed='1000' state='up'/ mtu size='1492'/ ... /interface Where @speed is negotiated link speed in Mbits per second, and state is the current NIC state (can be one of the following: unknown, notpresent, down, lowerlayerdown,testing, dormant, up). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- docs/schemas/basictypes.rng | 25 ++ docs/schemas/interface.rng | 1 + src/conf/device_conf.c | 62 + src/conf/device_conf.h | 27 ++- src/conf/interface_conf.c | 11 - src/conf/interface_conf.h | 2 + src/libvirt_private.syms| 4 ++ tests/interfaceschemadata/bridge-no-address.xml | 1 + tests/interfaceschemadata/bridge.xml| 1 + tests/interfaceschemadata/ethernet-dhcp.xml | 1 + 10 files changed, 133 insertions(+), 2 deletions(-) Already been ACK'd, but I will point out the interface.html.in hasn't been updated - later you update the nodedev.html.in file - so probably reasonable to do so again. While at it the consistency of using Mbits vs. Mb in the comment in device_conf.h The first issue has simple explanation - there's no interface.html.in yet. Yes we lack the virInterface XML documentation. The second one I'm fixing prior to push. Just so I understand - the reality is regardless of what the XML is on input - the code will still check/get/set the link state/speed - so this is mostly an output thing. Yes. So far this is only for getting the link speed state. Which brings up an interesting question. With recent issue with QoS on bridgeless networks on mind - should we make virInterface XML parsing actually reject any link/ since it's output element only? One could argue that we usually ignore unknown elements, but then link/ is not unknown element anymore. One way or another, it can be done in a separate patch. It's not like if on input someone had down for the state that we'd attempt to set the link down... or if the speed on input was 500, but the file had 1000 - there's no changing of the file. John Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 0/4] Expose link state speed
On 05.06.2014 17:39, Michal Privoznik wrote: While the 1/4 was ACKed, I'm sending int for completeness. And I've made a tiny change: the link speed is no longer unsigned long but unsigned int instead. That'll do too and it consumes less memory. Michal Privoznik (4): virInterface: Expose link state speed virnetdev: Introduce virNetDevGetLinkInfo interface_backend_udev: Implement link speed state node_device: Expose link state speed docs/formatnode.html.in | 6 ++ docs/schemas/basictypes.rng | 25 ++ docs/schemas/interface.rng | 1 + docs/schemas/nodedev.rng| 1 + src/Makefile.am | 4 +- src/conf/device_conf.c | 62 ++ src/conf/device_conf.h | 27 +- src/conf/interface_conf.c | 11 ++- src/conf/interface_conf.h | 2 + src/conf/node_device_conf.c | 7 +- src/conf/node_device_conf.h | 2 + src/interface/interface_backend_udev.c | 5 ++ src/libvirt_private.syms| 5 ++ src/node_device/node_device_driver.c| 11 ++- src/node_device/node_device_udev.c | 5 ++ src/util/virnetdev.c| 104 src/util/virnetdev.h| 6 ++ tests/interfaceschemadata/bridge-no-address.xml | 1 + tests/interfaceschemadata/bridge.xml| 1 + tests/interfaceschemadata/ethernet-dhcp.xml | 1 + 20 files changed, 280 insertions(+), 7 deletions(-) So I've fixed all the nits John found and pushed. Thanks! Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] virInterface: Expose link state speed
On Wed, Jun 11, 2014 at 11:19:14AM +0200, Michal Privoznik wrote: On 11.06.2014 03:13, John Ferlan wrote: On 06/05/2014 11:39 AM, Michal Privoznik wrote: Currently it is not possible to determine the speed of an interface and whether a link is actually detected from the API. Orchestrating platforms want to be able to determine when the link has failed and where multiple speeds may be available which one the interface is actually connected at. This commit introduces an extension to our interface XML (without implementation to interface driver backends): interface type='ethernet' name='eth0' start mode='none'/ mac address='aa:bb:cc:dd:ee:ff'/ link speed='1000' state='up'/ mtu size='1492'/ ... /interface Where @speed is negotiated link speed in Mbits per second, and state is the current NIC state (can be one of the following: unknown, notpresent, down, lowerlayerdown,testing, dormant, up). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- docs/schemas/basictypes.rng | 25 ++ docs/schemas/interface.rng | 1 + src/conf/device_conf.c | 62 + src/conf/device_conf.h | 27 ++- src/conf/interface_conf.c | 11 - src/conf/interface_conf.h | 2 + src/libvirt_private.syms| 4 ++ tests/interfaceschemadata/bridge-no-address.xml | 1 + tests/interfaceschemadata/bridge.xml| 1 + tests/interfaceschemadata/ethernet-dhcp.xml | 1 + 10 files changed, 133 insertions(+), 2 deletions(-) Already been ACK'd, but I will point out the interface.html.in hasn't been updated - later you update the nodedev.html.in file - so probably reasonable to do so again. While at it the consistency of using Mbits vs. Mb in the comment in device_conf.h The first issue has simple explanation - there's no interface.html.in yet. Yes we lack the virInterface XML documentation. The second one I'm fixing prior to push. Just so I understand - the reality is regardless of what the XML is on input - the code will still check/get/set the link state/speed - so this is mostly an output thing. Yes. So far this is only for getting the link speed state. Which brings up an interesting question. With recent issue with QoS on bridgeless networks on mind - should we make virInterface XML parsing actually reject any link/ since it's output element only? One could argue that we usually ignore unknown elements, but then link/ is not unknown element anymore. One way or another, it can be done in a separate patch. Apps must always be able to round-trip XML. ie they should be able to do DumpXML and then feed that back into DefineXML and have no functional change. Thus we must not reject link/ with an error when parsing, we must ignore it or honour it as appropriate for the desired semantics. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] Add unique_id to nodedev output
On Tue, Jun 10, 2014 at 03:03:55PM -0400, John Ferlan wrote:
Add an optional unique_id parameter to nodedev. Allows for easier lookup
and display of the unique_id value in order to document for use with
scsi_host code.
Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatnode.html.in| 11 +++
docs/schemas/nodedev.rng | 6 ++
src/conf/node_device_conf.c| 23
--
src/conf/node_device_conf.h| 1 +
src/node_device/node_device_linux_sysfs.c | 6 ++
src/test/test_driver.c | 5 +++--
.../pci_8086_27c5_scsi_host_0_unique_id.xml| 8
tests/nodedevxml2xmltest.c | 1 +
8 files changed, 53 insertions(+), 8 deletions(-)
create mode 100644
tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 76bf8af..cdf0277 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -173,6 +173,17 @@
dl
dtcodehost/code/dt
ddThe SCSI host number./dd
+ dtcodeunique_id/code/dt
+ ddOn input, this optionally provides the value from the
+'unique_id' file found in the scsi_host's directory. To
+view the values of all 'unique_id' files, use codefind -H
+/sys/class/scsi_host/host{0..9}/unique_id |
+xargs grep '[0-9]'/code. On output, if the unique_id
+file exists, the value from the file will be displayed.
+This can be used in order to help uniquely identify the
+scsi_host adapter in a a href=formatstorage.html
+Storage Pool/a. span class=sinceSince 1.2.6/span
+ /dd
Where is the data from the 'unique_id' coming from ? Is this since that
Linux makes up, or is it some standard attribute from the hardware ? It
would be desirable to document what it is in a way that's not simply
refering to Linux sysfs, since sysfs itself is mostly undocumented :-)
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] build: prefer -fstack-protector-strong to -all
On Wed, Jun 11, 2014 at 11:00:22AM +0200, Ján Tomko wrote: Check upfront if it's supported, to avoid putting both of them on the command line. --- m4/virt-compile-warnings.m4 | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 196afa7..6d632f9 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -156,6 +156,15 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ wantwarn=$wantwarn -Wframe-larger-than=4096 dnl wantwarn=$wantwarn -Wframe-larger-than=256 +AC_CACHE_CHECK([whether the C compiler supports stack-protector-strong], + [lv_cv_gcc_fstack_protector_strong], [ + save_CFLAGS=$CFLAGS + CFLAGS='-fstack-protector-strong -Werror' + AC_COMPILE_IFELSE([AC_LANG_PROGRAM([[]])], + [lv_cv_gcc_fstack_protector_strong=yes], + [lv_cv_gcc_fstack_protector_strong=no]) + CFLAGS=$save_CFLAGS]) This is really re-inventing the gnulib compiler arg checking which I don't think is desirable. @@ -164,13 +173,18 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl error: -fstack-protector not supported for this target [-Werror] ;; *-*-linux*) - dnl Fedora only uses -fstack-protector, but doesn't seem to - dnl be great overhead in adding -fstack-protector-all instead + dnl Prefer -fstack-protector-strong if it's available. + dnl There doesn't seem to be great overhead in adding + dnl -fstack-protector-all instead of -fstack-protector. dnl - dnl We also don't need ssp-buffer-size with -all, + dnl We also don't need ssp-buffer-size with -all or -strong, dnl since functions are protected regardless of buffer size. dnl wantwarn=$wantwarn --param=ssp-buffer-size=4 - wantwarn=$wantwarn -fstack-protector-all + if test $lv_cv_gcc_fstack_protector_strong = yes; then + wantwarn=$wantwarn -fstack-protector-strong + else + wantwarn=$wantwarn -fstack-protector-all + fi ;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle I'd suggest we only list 'wantwarn=$wantwarn -fstack-protector-strong' here. Then, after the 'gl_WARN_ADD' call has processed everything in $wantwarn we check to see if $WARNING_CFLAGS contains the desired -fstack-protector-strong arg and if not, we call gl_WARN_ADD for -fstack-protector-all Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] virInterface: Expose link state speed
On 06/11/2014 12:28 PM, Daniel P. Berrange wrote: On Wed, Jun 11, 2014 at 11:19:14AM +0200, Michal Privoznik wrote: On 11.06.2014 03:13, John Ferlan wrote: On 06/05/2014 11:39 AM, Michal Privoznik wrote: Currently it is not possible to determine the speed of an interface and whether a link is actually detected from the API. Orchestrating platforms want to be able to determine when the link has failed and where multiple speeds may be available which one the interface is actually connected at. This commit introduces an extension to our interface XML (without implementation to interface driver backends): interface type='ethernet' name='eth0' start mode='none'/ mac address='aa:bb:cc:dd:ee:ff'/ link speed='1000' state='up'/ mtu size='1492'/ ... /interface Where @speed is negotiated link speed in Mbits per second, and state is the current NIC state (can be one of the following: unknown, notpresent, down, lowerlayerdown,testing, dormant, up). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- docs/schemas/basictypes.rng | 25 ++ docs/schemas/interface.rng | 1 + src/conf/device_conf.c | 62 + src/conf/device_conf.h | 27 ++- src/conf/interface_conf.c | 11 - src/conf/interface_conf.h | 2 + src/libvirt_private.syms| 4 ++ tests/interfaceschemadata/bridge-no-address.xml | 1 + tests/interfaceschemadata/bridge.xml| 1 + tests/interfaceschemadata/ethernet-dhcp.xml | 1 + 10 files changed, 133 insertions(+), 2 deletions(-) Already been ACK'd, but I will point out the interface.html.in hasn't been updated - later you update the nodedev.html.in file - so probably reasonable to do so again. While at it the consistency of using Mbits vs. Mb in the comment in device_conf.h The first issue has simple explanation - there's no interface.html.in yet. Yes we lack the virInterface XML documentation. The second one I'm fixing prior to push. Just so I understand - the reality is regardless of what the XML is on input - the code will still check/get/set the link state/speed - so this is mostly an output thing. Yes. So far this is only for getting the link speed state. Which brings up an interesting question. With recent issue with QoS on bridgeless networks on mind - should we make virInterface XML parsing actually reject any link/ since it's output element only? One could argue that we usually ignore unknown elements, but then link/ is not unknown element anymore. One way or another, it can be done in a separate patch. Apps must always be able to round-trip XML. ie they should be able to do DumpXML and then feed that back into DefineXML and have no functional change. Thus we must not reject link/ with an error when parsing, we must ignore it or honour it as appropriate for the desired semantics. I agree that link should just be ignored when defining a domain. But as a general response to that last paragraph, there are cases where doing a virDomainGetXMLDesc() on an active domain (without the INACTIVE flag), followed by a virDomainDefine using that output, will *not* produce the same domain. The example at the front of my mind is what happens with an interface type='network', as related in my RFC email yesterday: https://www.redhat.com/archives/libvir-list/2014-June/msg00416.html but a more general problem is that if you run virDomainGetXMLDesc() with no INACTIVE flag, you will miss any previous edits to the domain definition since the last time it was started. This had actually been the cause of a long-running bug with virsh net-edit as a matter of fact. I had thought that what was required was the ability to roundtrip the *inactive* xml back into a define, not the status XML. If it's really a requirement that we be able to feed back the output of the status XML to get the exact same domain, then I think I've made a big mistake :-/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] virInterface: Expose link state speed
On Wed, Jun 11, 2014 at 01:04:02PM +0300, Laine Stump wrote: On 06/11/2014 12:28 PM, Daniel P. Berrange wrote: On Wed, Jun 11, 2014 at 11:19:14AM +0200, Michal Privoznik wrote: On 11.06.2014 03:13, John Ferlan wrote: On 06/05/2014 11:39 AM, Michal Privoznik wrote: Currently it is not possible to determine the speed of an interface and whether a link is actually detected from the API. Orchestrating platforms want to be able to determine when the link has failed and where multiple speeds may be available which one the interface is actually connected at. This commit introduces an extension to our interface XML (without implementation to interface driver backends): interface type='ethernet' name='eth0' start mode='none'/ mac address='aa:bb:cc:dd:ee:ff'/ link speed='1000' state='up'/ mtu size='1492'/ ... /interface Where @speed is negotiated link speed in Mbits per second, and state is the current NIC state (can be one of the following: unknown, notpresent, down, lowerlayerdown,testing, dormant, up). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- docs/schemas/basictypes.rng | 25 ++ docs/schemas/interface.rng | 1 + src/conf/device_conf.c | 62 + src/conf/device_conf.h | 27 ++- src/conf/interface_conf.c | 11 - src/conf/interface_conf.h | 2 + src/libvirt_private.syms| 4 ++ tests/interfaceschemadata/bridge-no-address.xml | 1 + tests/interfaceschemadata/bridge.xml| 1 + tests/interfaceschemadata/ethernet-dhcp.xml | 1 + 10 files changed, 133 insertions(+), 2 deletions(-) Already been ACK'd, but I will point out the interface.html.in hasn't been updated - later you update the nodedev.html.in file - so probably reasonable to do so again. While at it the consistency of using Mbits vs. Mb in the comment in device_conf.h The first issue has simple explanation - there's no interface.html.in yet. Yes we lack the virInterface XML documentation. The second one I'm fixing prior to push. Just so I understand - the reality is regardless of what the XML is on input - the code will still check/get/set the link state/speed - so this is mostly an output thing. Yes. So far this is only for getting the link speed state. Which brings up an interesting question. With recent issue with QoS on bridgeless networks on mind - should we make virInterface XML parsing actually reject any link/ since it's output element only? One could argue that we usually ignore unknown elements, but then link/ is not unknown element anymore. One way or another, it can be done in a separate patch. Apps must always be able to round-trip XML. ie they should be able to do DumpXML and then feed that back into DefineXML and have no functional change. Thus we must not reject link/ with an error when parsing, we must ignore it or honour it as appropriate for the desired semantics. I agree that link should just be ignored when defining a domain. But as a general response to that last paragraph, there are cases where doing a virDomainGetXMLDesc() on an active domain (without the INACTIVE flag), followed by a virDomainDefine using that output, will *not* produce the same domain. The example at the front of my mind is what happens with an interface type='network', as related in my RFC email yesterday: https://www.redhat.com/archives/libvir-list/2014-June/msg00416.html but a more general problem is that if you run virDomainGetXMLDesc() with no INACTIVE flag, you will miss any previous edits to the domain definition since the last time it was started. This had actually been the cause of a long-running bug with virsh net-edit as a matter of fact. I had thought that what was required was the ability to roundtrip the *inactive* xml back into a define, not the status XML. If it's really a requirement that we be able to feed back the output of the status XML to get the exact same domain, then I think I've made a big mistake :-/ Yes, you're right actually. I should have clarified I meant inactive XML. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] vmware: make version parsing more robust
On 06/09/2014 10:12 AM, Jean-Baptiste Rouault wrote: On Monday 28 April 2014 13:46:54 Ján Tomko wrote: On 04/09/2014 11:59 AM, Jean-Baptiste Rouault wrote: Since commit d69415d4, vmware version is parsed from both stdout and stderr. This patch makes version parsing work even if there is garbage (libvirt debug messages for example) in the command output. For libvirt's debug messages, we have virLogProbablyLogMessage that can check if the message matches libvirt's format. Should we really ignore all garbage? Honestly I don't know, but as of today the only garbage I saw there was from libvirt. Apart from the version string, I don't think there could be useful information in the command output. It's probably not worth the trouble. ACK to your patch, I will push later today. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] virInterface: Expose link state speed
On 06/11/2014 05:19 AM, Michal Privoznik wrote: On 11.06.2014 03:13, John Ferlan wrote: On 06/05/2014 11:39 AM, Michal Privoznik wrote: Currently it is not possible to determine the speed of an interface and whether a link is actually detected from the API. Orchestrating platforms want to be able to determine when the link has failed and where multiple speeds may be available which one the interface is actually connected at. This commit introduces an extension to our interface XML (without implementation to interface driver backends): interface type='ethernet' name='eth0' start mode='none'/ mac address='aa:bb:cc:dd:ee:ff'/ link speed='1000' state='up'/ mtu size='1492'/ ... /interface Where @speed is negotiated link speed in Mbits per second, and state is the current NIC state (can be one of the following: unknown, notpresent, down, lowerlayerdown,testing, dormant, up). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- docs/schemas/basictypes.rng | 25 ++ docs/schemas/interface.rng | 1 + src/conf/device_conf.c | 62 + src/conf/device_conf.h | 27 ++- src/conf/interface_conf.c | 11 - src/conf/interface_conf.h | 2 + src/libvirt_private.syms| 4 ++ tests/interfaceschemadata/bridge-no-address.xml | 1 + tests/interfaceschemadata/bridge.xml| 1 + tests/interfaceschemadata/ethernet-dhcp.xml | 1 + 10 files changed, 133 insertions(+), 2 deletions(-) Already been ACK'd, but I will point out the interface.html.in hasn't been updated - later you update the nodedev.html.in file - so probably reasonable to do so again. While at it the consistency of using Mbits vs. Mb in the comment in device_conf.h The first issue has simple explanation - there's no interface.html.in yet. Yes we lack the virInterface XML documentation. The second one I'm fixing prior to push. So - I was just too lazy/tired to look last night, but formatdomain.html.in has an Network interfaces sub-section... Perhaps woefully under described though... John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] virInterface: Expose link state speed
On 06/11/2014 01:11 PM, Daniel P. Berrange wrote: On Wed, Jun 11, 2014 at 01:04:02PM +0300, Laine Stump wrote: I had thought that what was required was the ability to roundtrip the *inactive* xml back into a define, not the status XML. If it's really a requirement that we be able to feed back the output of the status XML to get the exact same domain, then I think I've made a big mistake :-/ Yes, you're right actually. I should have clarified I meant inactive XML. Whew! I feel much better now :-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/4] virInterface: Expose link state speed
On Wed, Jun 11, 2014 at 06:46:25AM -0400, John Ferlan wrote: On 06/11/2014 05:19 AM, Michal Privoznik wrote: On 11.06.2014 03:13, John Ferlan wrote: On 06/05/2014 11:39 AM, Michal Privoznik wrote: Currently it is not possible to determine the speed of an interface and whether a link is actually detected from the API. Orchestrating platforms want to be able to determine when the link has failed and where multiple speeds may be available which one the interface is actually connected at. This commit introduces an extension to our interface XML (without implementation to interface driver backends): interface type='ethernet' name='eth0' start mode='none'/ mac address='aa:bb:cc:dd:ee:ff'/ link speed='1000' state='up'/ mtu size='1492'/ ... /interface Where @speed is negotiated link speed in Mbits per second, and state is the current NIC state (can be one of the following: unknown, notpresent, down, lowerlayerdown,testing, dormant, up). Signed-off-by: Michal Privoznik mpriv...@redhat.com --- docs/schemas/basictypes.rng | 25 ++ docs/schemas/interface.rng | 1 + src/conf/device_conf.c | 62 + src/conf/device_conf.h | 27 ++- src/conf/interface_conf.c | 11 - src/conf/interface_conf.h | 2 + src/libvirt_private.syms| 4 ++ tests/interfaceschemadata/bridge-no-address.xml | 1 + tests/interfaceschemadata/bridge.xml| 1 + tests/interfaceschemadata/ethernet-dhcp.xml | 1 + 10 files changed, 133 insertions(+), 2 deletions(-) Already been ACK'd, but I will point out the interface.html.in hasn't been updated - later you update the nodedev.html.in file - so probably reasonable to do so again. While at it the consistency of using Mbits vs. Mb in the comment in device_conf.h The first issue has simple explanation - there's no interface.html.in yet. Yes we lack the virInterface XML documentation. The second one I'm fixing prior to push. So - I was just too lazy/tired to look last night, but formatdomain.html.in has an Network interfaces sub-section... Perhaps woefully under described though... NB the network interfaces schema for virDomain (as documented in the formatdomain.html page) is different from the network interfaces schema for virInterface (as not documented :-) Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V8 2/2] libxl: add migration support
This patch has resulted in a new Coverity warnings (looking at them was
just lower on my list of things to do lately)...
Anyway - see libxlDoMigrateReceive() and libxlDomainMigrationFinish()
for the details...
John
On 06/04/2014 04:35 PM, Jim Fehlig wrote:
This patch adds initial migration support to the libxl driver,
using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration
functions.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
po/POTFILES.in | 1 +
src/Makefile.am | 3 +-
src/libxl/libxl_conf.h | 6 +
src/libxl/libxl_domain.h| 1 +
src/libxl/libxl_driver.c| 235 ++
src/libxl/libxl_migration.c | 585
src/libxl/libxl_migration.h | 79 ++
7 files changed, 909 insertions(+), 1 deletion(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index cff92d9..2ee7225 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -74,6 +74,7 @@ src/lxc/lxc_process.c
src/libxl/libxl_domain.c
src/libxl/libxl_driver.c
src/libxl/libxl_conf.c
+src/libxl/libxl_migration.c
src/network/bridge_driver.c
src/network/bridge_driver_linux.c
src/network/leaseshelper.c
diff --git a/src/Makefile.am b/src/Makefile.am
index d82ca26..01af164 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -707,7 +707,8 @@ XENAPI_DRIVER_SOURCES =
\
LIBXL_DRIVER_SOURCES = \
libxl/libxl_conf.c libxl/libxl_conf.h \
libxl/libxl_domain.c libxl/libxl_domain.h \
- libxl/libxl_driver.c libxl/libxl_driver.h
+ libxl/libxl_driver.c libxl/libxl_driver.h \
+ libxl/libxl_migration.c libxl/libxl_migration.h
UML_DRIVER_SOURCES = \
uml/uml_conf.c uml/uml_conf.h \
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 433d6da..6aa36d2 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -43,6 +43,9 @@
# define LIBXL_VNC_PORT_MIN 5900
# define LIBXL_VNC_PORT_MAX 65535
+# define LIBXL_MIGRATION_PORT_MIN 49152
+# define LIBXL_MIGRATION_PORT_MAX 49216
+
# define LIBXL_CONFIG_DIR SYSCONFDIR /libvirt/libxl
# define LIBXL_AUTOSTART_DIR LIBXL_CONFIG_DIR /autostart
# define LIBXL_STATE_DIR LOCALSTATEDIR /run/libvirt/libxl
@@ -115,6 +118,9 @@ struct _libxlDriverPrivate {
/* Immutable pointer, self-locking APIs */
virPortAllocatorPtr reservedVNCPorts;
+/* Immutable pointer, self-locking APIs */
+virPortAllocatorPtr migrationPorts;
+
/* Immutable pointer, lockless APIs*/
virSysinfoDefPtr hostsysinfo;
};
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index 6939008..f459fdf 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -69,6 +69,7 @@ struct _libxlDomainObjPrivate {
virChrdevsPtr devs;
libxl_evgen_domain_death *deathW;
libxlDriverPrivatePtr driver;
+unsigned short migrationPort;
struct libxlDomainJobObj job;
};
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 515d5c9..9feacb1 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -45,6 +45,7 @@
#include libxl_domain.h
#include libxl_driver.h
#include libxl_conf.h
+#include libxl_migration.h
#include xen_xm.h
#include xen_sxpr.h
#include virtypedparam.h
@@ -209,6 +210,7 @@ libxlStateCleanup(void)
virObjectUnref(libxl_driver-xmlopt);
virObjectUnref(libxl_driver-domains);
virObjectUnref(libxl_driver-reservedVNCPorts);
+virObjectUnref(libxl_driver-migrationPorts);
virObjectEventStateFree(libxl_driver-domainEventState);
virSysinfoDefFree(libxl_driver-hostsysinfo);
@@ -301,6 +303,13 @@ libxlStateInitialize(bool privileged,
LIBXL_VNC_PORT_MAX)))
goto error;
+/* Allocate bitmap for migration port reservation */
+if (!(libxl_driver-migrationPorts =
+ virPortAllocatorNew(_(migration),
+ LIBXL_MIGRATION_PORT_MIN,
+ LIBXL_MIGRATION_PORT_MAX)))
+goto error;
+
if (!(libxl_driver-domains = virDomainObjListNew()))
goto error;
@@ -4153,6 +4162,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int
feature)
switch (feature) {
case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
+case VIR_DRV_FEATURE_MIGRATION_PARAMS:
return 1;
default:
return 0;
@@ -4331,6 +4341,226 @@ libxlNodeDeviceReset(virNodeDevicePtr dev)
return ret;
}
+static char *
+libxlDomainMigrateBegin3Params(virDomainPtr domain,
+ virTypedParameterPtr params,
+ int nparams,
+ char **cookieout ATTRIBUTE_UNUSED,
+
[libvirt] [PATCH] virnetdev: Use ifname in virNetDevGetLinkInfo
If we're compiling on non-Linux platform, the virNetDevGetLinkInfo()
is a dummy function which barely logs debug message that getting link
info is not supported. However, while the debug message was prepared
for printing the interface name too, I actually forgot to pass the
variable which resulted in build error on platforms like mingw or
FreeBSD.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
Notes:
Pushed under build breaker rule.
src/util/virnetdev.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index cbf086e..6f3a202 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -1921,7 +1921,8 @@ virNetDevGetLinkInfo(const char *ifname,
virInterfaceLinkPtr lnk)
{
/* Port me */
-VIR_DEBUG(Getting link info on %s is not implemented on this platform);
+VIR_DEBUG(Getting link info on %s is not implemented on this platform,
+ ifname);
lnk-speed = lnk-state = 0;
return 0;
}
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 06/21] tests: virstoragetest: Remove expBackingStore field
Now that we changed ordering of the stored metadata so that the backing
store is described by the child element the test should reflect this
change too.
Remove the expected backing store field as it's actually described by
the next element in the backing chain, so there's no need for
duplication.
---
tests/virstoragetest.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 5cea929..b7383c0 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -282,7 +282,6 @@ testPrepImages(void)
typedef struct _testFileData testFileData;
struct _testFileData
{
-const char *expBackingStore;
const char *expBackingStoreRaw;
unsigned long long expCapacity;
bool expEncrypted;
@@ -314,12 +313,11 @@ struct testChainData
static const char testStorageChainFormat[] =
-store: %s\n
+path:%s\n
backingStoreRaw: %s\n
capacity: %lld\n
encryption: %d\n
relPath:%s\n
-path:%s\n
relDir:%s\n
type:%d\n
format:%d\n;
@@ -386,23 +384,21 @@ testStorageChain(const void *args)
: data-files[i]-relDirRel;
if (virAsprintf(expect,
testStorageChainFormat,
-NULLSTR(data-files[i]-expBackingStore),
+NULLSTR(data-files[i]-path),
NULLSTR(data-files[i]-expBackingStoreRaw),
data-files[i]-expCapacity,
data-files[i]-expEncrypted,
NULLSTR(expPath),
-NULLSTR(data-files[i]-path),
NULLSTR(expRelDir),
data-files[i]-type,
data-files[i]-format) 0 ||
virAsprintf(actual,
testStorageChainFormat,
-NULLSTR(elt-backingStore ? elt-backingStore-path :
NULL),
+NULLSTR(elt-path),
NULLSTR(elt-backingStoreRaw),
elt-capacity,
!!elt-encryption,
NULLSTR(elt-relPath),
-NULLSTR(elt-path),
NULLSTR(elt-relDir),
elt-type,
elt-format) 0) {
@@ -762,7 +758,6 @@ mymain(void)
/* Qcow2 file with relative raw backing, format provided */
raw.pathAbs = raw;
testFileData qcow2 = {
-.expBackingStore = canonraw,
.expBackingStoreRaw = raw,
.expCapacity = 1024,
.pathRel = qcow2,
@@ -818,7 +813,6 @@ mymain(void)
/* Wrapped file access */
testFileData wrap = {
-.expBackingStore = canonqcow2,
.expBackingStoreRaw = absqcow2,
.expCapacity = 1024,
.pathRel = wrap,
@@ -854,7 +848,6 @@ mymain(void)
/* Qcow2 file with raw as absolute backing, backing format omitted */
testFileData wrap_as_raw = {
-.expBackingStore = canonqcow2,
.expBackingStoreRaw = absqcow2,
.expCapacity = 1024,
.pathRel = wrap,
@@ -878,7 +871,6 @@ mymain(void)
qcow2, NULL);
if (virCommandRun(cmd, NULL) 0)
ret = -1;
-qcow2.expBackingStore = NULL;
qcow2.expBackingStoreRaw = datadir /bogus;
qcow2.pathRel = qcow2;
qcow2.relDirRel = .;
@@ -911,7 +903,6 @@ mymain(void)
qcow2, NULL);
if (virCommandRun(cmd, NULL) 0)
ret = -1;
-qcow2.expBackingStore = blah;
qcow2.expBackingStoreRaw = nbd:example.org:6000:exportname=blah;
/* Qcow2 file with backing protocol instead of file */
@@ -932,7 +923,6 @@ mymain(void)
/* qed file */
testFileData qed = {
-.expBackingStore = canonraw,
.expBackingStoreRaw = absraw,
.expCapacity = 1024,
.pathRel = qed,
@@ -997,7 +987,6 @@ mymain(void)
/* Behavior of symlinks to qcow2 with relative backing files */
testFileData link1 = {
-.expBackingStore = canonraw,
.expBackingStoreRaw = ../raw,
.expCapacity = 1024,
.pathRel = ../sub/link1,
@@ -1009,7 +998,6 @@ mymain(void)
.format = VIR_STORAGE_FILE_QCOW2,
};
testFileData link2 = {
-.expBackingStore = canonqcow2,
.expBackingStoreRaw = ../sub/link1,
.expCapacity = 1024,
.pathRel = sub/link2,
@@ -1037,7 +1025,6 @@ mymain(void)
-F, qcow2, -b, qcow2, qcow2, NULL);
if (virCommandRun(cmd, NULL) 0)
ret = -1;
-qcow2.expBackingStore = NULL;
qcow2.expBackingStoreRaw = qcow2;
/* Behavior of an infinite loop chain */
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 19/21] lib: Introduce flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE
Introduce flag for the block rebase API to allow the rebase operation to
leave the chain relatively addressed. Also adds a virsh switch to enable
this behavior.
---
include/libvirt/libvirt.h.in | 2 ++
src/libvirt.c| 5 +
tools/virsh-domain.c | 22 +++---
tools/virsh.pod | 4
4 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index bacdf57..44efd60 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2573,6 +2573,8 @@ typedef enum {
file for a copy */
VIR_DOMAIN_BLOCK_REBASE_COPY_RAW = 1 2, /* Make destination file raw */
VIR_DOMAIN_BLOCK_REBASE_COPY = 1 3, /* Start a copy job */
+VIR_DOMAIN_BLOCK_REBASE_RELATIVE = 1 4, /* Keep backing chain relative
+ if possible */
} virDomainBlockRebaseFlags;
int virDomainBlockRebase(virDomainPtr dom, const char *disk,
diff --git a/src/libvirt.c b/src/libvirt.c
index 21ef41f..a5f9547 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -19716,6 +19716,11 @@ virDomainBlockPull(virDomainPtr dom, const char *disk,
* exists. If the job is aborted, a new one can be started later to
* resume from the same point.
*
+ * If @flags contains VIR_DOMAIN_BLOCK_REBASE_RELATIVE, the name recorded
+ * into the overlay of the @base image as path to the new backing file
+ * will be kept relative to other images in case the backing chain was
+ * using relative names.
+ *
* When @flags includes VIR_DOMAIN_BLOCK_REBASE_COPY, this starts a copy,
* where @base must be the name of a new file to copy the chain to. By
* default, the copy will pull the entire source chain into the destination
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f26a133..e9162db 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1479,10 +1479,14 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
case VSH_CMD_BLOCK_JOB_PULL:
if (vshCommandOptStringReq(ctl, cmd, base, base) 0)
goto cleanup;
-if (base)
-ret = virDomainBlockRebase(dom, path, base, bandwidth, 0);
-else
+if (base) {
+ if (vshCommandOptBool(cmd, keep-relative))
+ flags |= VIR_DOMAIN_BLOCK_REBASE_RELATIVE;
+
+ret = virDomainBlockRebase(dom, path, base, bandwidth, flags);
+} else {
ret = virDomainBlockPull(dom, path, bandwidth, 0);
+}
break;
case VSH_CMD_BLOCK_JOB_COMMIT:
if (vshCommandOptStringReq(ctl, cmd, base, base) 0 ||
@@ -2072,6 +2076,11 @@ static const vshCmdOptDef opts_block_pull[] = {
.type = VSH_OT_BOOL,
.help = N_(with --wait, don't wait for cancel to finish)
},
+{.name = keep-relative,
+ .type = VSH_OT_BOOL,
+ .help = N_(keep the backing chain relative if it was relatively
+referenced if it was before)
+},
{.name = NULL}
};
@@ -2092,6 +2101,13 @@ cmdBlockPull(vshControl *ctl, const vshCmd *cmd)
bool quit = false;
int abort_flags = 0;
+if (vshCommandOptBool(cmd, keep-relative)
+!vshCommandOptBool(cmd, base)) {
+vshError(ctl, %s, _(--keep-relative is supported only with partial
+ pull operations with --base specified));
+return false;
+}
+
if (blocking) {
if (vshCommandOptTimeoutToMs(ctl, cmd, timeout) 0)
return false;
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 5816c0b..84a60a7 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -846,6 +846,7 @@ Ibandwidth specifies copying bandwidth limit in MiB/s.
=item Bblockpull Idomain Ipath [Ibandwidth] [Ibase]
[I--wait [I--verbose] [I--timeout Bseconds] [I--async]]
+[I--keep-relative]
Populate a disk from its backing image chain. By default, this command
flattens the entire chain; but if Ibase is specified, containing the
@@ -865,6 +866,9 @@ is triggered, I--async will return control to the user as
fast as
possible, otherwise the command may continue to block a little while
longer until the job is done cleaning up.
+Using the I--keep-relative flag will try to keep the backing chain names
+relative (if they were relative before).
+
Ipath specifies fully-qualified path of the disk; it corresponds
to a unique target name (target dev='name'/) or source file (source
file='name'/) for one of the disk devices attached to Idomain (see
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 01/21] security: Don't skip labelling for network disks
A network disk might actually be backed by local storage. Also the path iterator actually handles networked disks well now so remove the code that skips the labelling in dac and selinux security driver. --- src/security/security_dac.c | 3 --- src/security/security_selinux.c | 3 --- 2 files changed, 6 deletions(-) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 015b699..9d5c25b 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -333,9 +333,6 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr mgr, if (!priv-dynamicOwnership) return 0; -if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK) -return 0; - secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME); if (secdef secdef-norelabel) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 008c58c..228e5cb 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1255,9 +1255,6 @@ virSecuritySELinuxSetSecurityImageLabel(virSecurityManagerPtr mgr, if (!cbdata.secdef || cbdata.secdef-norelabel) return 0; -if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK) -return 0; - return virDomainDiskDefForeachPath(disk, true, virSecuritySELinuxSetSecurityFileLabel, -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 05/21] util: storage: Add helper to resolve relative path difference
This patch introduces a function that will allow us to resolve a
relative difference between two elements of a disk backing chain. This
fucntion will be used to allow relative block commit and block pull
where we need to specify the new relative name of the image to qemu.
This patch also adds unit tests for the function to verify that it works
correctly.
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 77
src/util/virstoragefile.h | 4 ++
tests/virstoragetest.c| 146 ++
4 files changed, 228 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 7671730..773b208 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1875,6 +1875,7 @@ virStorageFileGetLVMKey;
virStorageFileGetMetadataFromBuf;
virStorageFileGetMetadataFromFD;
virStorageFileGetMetadataInternal;
+virStorageFileGetRelativeBackingPath;
virStorageFileGetSCSIKey;
virStorageFileIsClusterFS;
virStorageFileParseChainIndex;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index ef69bf3..ce1fc86 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2123,3 +2123,80 @@ virStorageFileCanonicalizePath(const char *path,
return ret;
}
+
+
+static char *
+virStorageFileRemoveLastPathComponent(const char *path)
+{
+char *tmp;
+char *ret;
+
+if (VIR_STRDUP(ret, path ? path : ) 0)
+return NULL;
+
+if ((tmp = strrchr(ret, '/')))
+tmp[1] = '\0';
+else
+ret[0] = '\0';
+
+return ret;
+}
+
+
+/*
+ * virStorageFileGetRelativeBackingPath:
+ *
+ * Resolve relative path to be written to the overlay of @top image when
+ * collapsing the backing chain between @top and @base.
+ *
+ * Returns 0 on success; 1 if backing chain isn't relative and -1 on error.
+ */
+int
+virStorageFileGetRelativeBackingPath(virStorageSourcePtr top,
+ virStorageSourcePtr base,
+ char **relpath)
+{
+virStorageSourcePtr next;
+char *tmp = NULL;
+char *path = NULL;
+char ret = -1;
+
+*relpath = NULL;
+
+for (next = top; next; next = next-backingStore) {
+if (!next-backingRelative || !next-relPath) {
+ret = 1;
+goto cleanup;
+}
+
+if (!(tmp = virStorageFileRemoveLastPathComponent(path)))
+goto cleanup;
+
+VIR_FREE(path);
+
+if (virAsprintf(path, %s%s, tmp, next-relPath) 0)
+goto cleanup;
+
+VIR_FREE(tmp);
+
+if (next == base)
+break;
+}
+
+if (next != base) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(failed to resolve relative backing name:
+ base image is not in backing chain));
+goto cleanup;
+}
+
+*relpath = path;
+path = NULL;
+
+ret = 0;
+
+ cleanup:
+VIR_FREE(path);
+VIR_FREE(tmp);
+return ret;
+}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index fd5c89e..ff8130d 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -333,4 +333,8 @@ char *virStorageFileCanonicalizePath(const char *path,
virStorageFileSimplifyPathReadlinkCallback cb,
void *cbdata);
+int virStorageFileGetRelativeBackingPath(virStorageSourcePtr from,
+ virStorageSourcePtr to,
+ char **relpath);
+
#endif /* __VIR_STORAGE_FILE_H__ */
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 8111f58..5cea929 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -588,6 +588,104 @@ testPathCanonicalize(const void *args)
return ret;
}
+virStorageSource backingchain[12];
+
+static void
+testPathRelativePrepare(void)
+{
+size_t i;
+
+for (i = 0; i ARRAY_CARDINALITY(backingchain); i++) {
+if (i ARRAY_CARDINALITY(backingchain) - 1)
+backingchain[i].backingStore = backingchain[i+1];
+else
+backingchain[i].backingStore = NULL;
+
+backingchain[i].backingRelative = true;
+}
+
+/* normal relative backing chain */
+backingchain[0].path = (char *) /path/to/some/img;
+backingchain[0].relPath = (char *) /path/to/some/img;
+backingchain[0].backingRelative = false;
+
+backingchain[1].path = (char *) /path/to/some/asdf;
+backingchain[1].relPath = (char *) asdf;
+
+backingchain[2].path = (char *) /path/to/some/test;
+backingchain[2].relPath = (char *) test;
+
+backingchain[3].path = (char *) /path/to/some/blah;
+backingchain[3].relPath = (char *) blah;
+
+/* ovirt's backing chain */
+backingchain[4].path = (char *) /path/to/volume/image1;
+backingchain[4].relPath = (char *) /path/to/volume/image1;
+backingchain[4].backingRelative = false;
+
+
[libvirt] [PATCHv4 00/21] block pull/commit on gluster volumes with relative backing
Peter Krempa (21): security: Don't skip labelling for network disks util: string: Add helper to free non-NULL terminated string arrays util: storagefile: Introduce universal function to canonicalize paths storage: gluster: Add backend to return unique storage file path util: storage: Add helper to resolve relative path difference tests: virstoragetest: Remove expBackingStore field tests: virstoragetest: Fix output when hitting errors storage: Store relative path only for relatively backed storage tests: virstoragetest: Remove now unused pathAbs util: storage: Remove now redundant backingRelative from virStorageSource tests: virstoragetest: Don't test relative start of backing chains tests: virstoragetest: Remove unneeded relative test plumbing storage: Don't canonicalize paths unnecessarily storage: Don't store parent directory of an image explicitly qemu: caps: Add capability for change-backing-file command qemu: monitor: Add argument for specifying backing name for block commit qemu: monitor: Add support for backing name specification for block-stream lib: Introduce flag VIR_DOMAIN_BLOCK_COMMIT_RELATIVE lib: Introduce flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE qemu: Add support for networked disks for block commit qemu: Add support for networked disks for block pull/block rebase include/libvirt/libvirt.h.in | 6 + src/libvirt.c | 10 + src/libvirt_private.syms | 3 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_driver.c| 85 ++- src/qemu/qemu_migration.c | 6 +- src/qemu/qemu_monitor.c | 21 +- src/qemu/qemu_monitor.h | 4 +- src/qemu/qemu_monitor_json.c | 17 ++ src/qemu/qemu_monitor_json.h | 2 + src/security/security_dac.c | 3 - src/security/security_selinux.c | 3 - src/storage/storage_backend_gluster.c | 80 ++ src/storage/storage_driver.c | 15 +- src/util/virstoragefile.c | 392 +++-- src/util/virstoragefile.h | 20 +- src/util/virstring.c | 20 ++ src/util/virstring.h | 1 + tests/qemumonitorjsontest.c | 2 +- tests/virstoragetest.c| 450 +- tools/virsh-domain.c | 29 ++- tools/virsh.pod | 10 +- 23 files changed, 883 insertions(+), 299 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 15/21] qemu: caps: Add capability for change-backing-file command
This command allows to change the backing file name recorded in the
metadata of a qcow (or other) image. The capability also notifies that
the block-stream and block-commit commands understand the
backing-file attribute.
---
src/qemu/qemu_capabilities.c | 2 ++
src/qemu/qemu_capabilities.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 08c3d04..05613e5 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -256,6 +256,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
usb-kbd, /* 165 */
host-pci-multidomain,
msg-timestamp,
+ change-backing-file,
);
@@ -1382,6 +1383,7 @@ struct virQEMUCapsStringFlags virQEMUCapsCommands[] = {
{ blockdev-snapshot-sync, QEMU_CAPS_DISK_SNAPSHOT },
{ add-fd, QEMU_CAPS_ADD_FD },
{ nbd-server-start, QEMU_CAPS_NBD_SERVER },
+{ change-backing-file, QEMU_CAPS_CHANGE_BACKING_FILE },
};
struct virQEMUCapsStringFlags virQEMUCapsEvents[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d755caa..6b4d7c4 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -206,6 +206,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_DEVICE_USB_KBD = 165, /* -device usb-kbd */
QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain 0 in host pci
address */
QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */
+QEMU_CAPS_CHANGE_BACKING_FILE = 168, /* change name of backing file in
metadata */
QEMU_CAPS_LAST, /* this must always be the last item */
};
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 07/21] tests: virstoragetest: Fix output when hitting errors
When the test is failing but the debug output isn't enabled the
resulting line would look ugly like and would not contain the actual
difference.
TEST: virstoragetest
.chain member 1!chain member 1!chain member 1!
Store the member index in the actual checked string to hide this problem
---
tests/virstoragetest.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index b7383c0..6068612 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -313,6 +313,7 @@ struct testChainData
static const char testStorageChainFormat[] =
+chain member: %zu\n
path:%s\n
backingStoreRaw: %s\n
capacity: %lld\n
@@ -383,7 +384,7 @@ testStorageChain(const void *args)
expRelDir = isAbs ? data-files[i]-relDirAbs
: data-files[i]-relDirRel;
if (virAsprintf(expect,
-testStorageChainFormat,
+testStorageChainFormat, i,
NULLSTR(data-files[i]-path),
NULLSTR(data-files[i]-expBackingStoreRaw),
data-files[i]-expCapacity,
@@ -393,7 +394,7 @@ testStorageChain(const void *args)
data-files[i]-type,
data-files[i]-format) 0 ||
virAsprintf(actual,
-testStorageChainFormat,
+testStorageChainFormat, i,
NULLSTR(elt-path),
NULLSTR(elt-backingStoreRaw),
elt-capacity,
@@ -407,7 +408,6 @@ testStorageChain(const void *args)
goto cleanup;
}
if (STRNEQ(expect, actual)) {
-fprintf(stderr, chain member %zu, i);
virtTestDifference(stderr, expect, actual);
VIR_FREE(expect);
VIR_FREE(actual);
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 13/21] storage: Don't canonicalize paths unnecessarily
Store backing chain paths as non-canonical. The canonicalization step
will be already taken. This will allow to avoid storing unnecessary
amounts of data.
---
src/util/virstoragefile.c | 33 ++---
tests/virstoragetest.c| 10 +-
2 files changed, 11 insertions(+), 32 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 3b0a4ab..2524efa 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1044,25 +1044,17 @@ virStorageFileGetMetadataFromFD(const char *path,
int *backingFormat)
{
-virStorageSourcePtr ret = NULL;
-char *canonPath = NULL;
-
-if (!(canonPath = canonicalize_file_name(path))) {
-virReportSystemError(errno, _(unable to resolve '%s'), path);
-goto cleanup;
-}
+virStorageSourcePtr ret;
-if (!(ret = virStorageFileMetadataNew(canonPath, format)))
-goto cleanup;
+if (!(ret = virStorageFileMetadataNew(path, format)))
+return NULL;
if (virStorageFileGetMetadataFromFDInternal(ret, fd, backingFormat) 0) {
virStorageSourceFree(ret);
-ret = NULL;
+return NULL;
}
- cleanup:
-VIR_FREE(canonPath);
return ret;
}
@@ -1611,15 +1603,6 @@
virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
virReportOOMError();
goto error;
}
-
-/* XXX we don't currently need to store the canonical path but the
- * change would break the test suite. Get rid of this later */
-char *tmp = ret-path;
-if (!(ret-path = canonicalize_file_name(tmp))) {
-ret-path = tmp;
-tmp = NULL;
-}
-VIR_FREE(tmp);
} else {
ret-type = VIR_STORAGE_TYPE_NETWORK;
@@ -1858,12 +1841,8 @@ virStorageSourceNewFromBackingAbsolute(const char *path)
goto error;
}
-/* XXX we don't currently need to store the canonical path but the
- * change would break the test suite. Get rid of this later */
-if (!(ret-path = canonicalize_file_name(path))) {
-if (VIR_STRDUP(ret-path, path) 0)
-goto error;
-}
+if (VIR_STRDUP(ret-path, path) 0)
+goto error;
} else {
ret-type = VIR_STORAGE_TYPE_NETWORK;
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 782f3a8..2511418 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -121,10 +121,8 @@ testStorageFileGetMetadata(const char *path,
goto error;
}
-if (!(ret-path = canonicalize_file_name(path))) {
-virReportError(VIR_ERR_INTERNAL_ERROR, failed to resolve '%s', path);
+if (VIR_STRDUP(ret-path, path) 0)
goto error;
-}
if (virStorageFileGetMetadata(ret, uid, gid, allow_probe) 0)
goto error;
@@ -901,7 +899,7 @@ mymain(void)
.expBackingStoreRaw = ../raw,
.expCapacity = 1024,
.pathRel = ../sub/link1,
-.path = canonqcow2,
+.path = datadir /sub/../sub/link1,
.relDir = datadir /sub/../sub,
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
@@ -909,11 +907,13 @@ mymain(void)
testFileData link2 = {
.expBackingStoreRaw = ../sub/link1,
.expCapacity = 1024,
-.path = canonwrap,
+.path = abslink2,
.relDir = datadir /sub,
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
};
+
+raw.path = datadir /sub/../sub/../raw;
raw.pathRel = ../raw;
raw.relDir = datadir /sub/../sub/..;
TEST_CHAIN(15, abslink2, VIR_STORAGE_FILE_QCOW2,
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 02/21] util: string: Add helper to free non-NULL terminated string arrays
Sometimes the length of the string list is known but the array isn't
NULL terminated. Add helper to free the array in such cases.
---
src/libvirt_private.syms | 1 +
src/util/virstring.c | 20
src/util/virstring.h | 1 +
3 files changed, 22 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 122c572..f69cd1c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1909,6 +1909,7 @@ virStrcpy;
virStrdup;
virStringArrayHasString;
virStringFreeList;
+virStringFreeListCount;
virStringJoin;
virStringListLength;
virStringReplace;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 6dcc7a8..35b99a5 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -187,6 +187,26 @@ void virStringFreeList(char **strings)
}
+/**
+ * virStringFreeListCount:
+ * @strings: array of strings to free
+ * @count: number of elements in the array
+ *
+ * Frees a string array of @count length.
+ */
+void
+virStringFreeListCount(char **strings,
+ size_t count)
+{
+size_t i;
+
+for (i = 0; i count; i++)
+VIR_FREE(strings[i]);
+
+VIR_FREE(strings);
+}
+
+
bool
virStringArrayHasString(char **strings, const char *needle)
{
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 6ddcff5..df25441 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -42,6 +42,7 @@ char *virStringJoin(const char **strings,
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
void virStringFreeList(char **strings);
+void virStringFreeListCount(char **strings, size_t count);
bool virStringArrayHasString(char **strings, const char *needle);
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 16/21] qemu: monitor: Add argument for specifying backing name for block commit
To allow changing the name that is recorded in the overlay of the TOP
image used in a block commit operation, we need to specify the backing
name to qemu. This is done via the backing-file attribute to the
block-commit command.
---
src/qemu/qemu_driver.c | 1 +
src/qemu/qemu_monitor.c | 9 ++---
src/qemu/qemu_monitor.h | 1 +
src/qemu/qemu_monitor_json.c | 2 ++
src/qemu/qemu_monitor_json.h | 1 +
tests/qemumonitorjsontest.c | 2 +-
6 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1191255..61f2beb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15605,6 +15605,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
ret = qemuMonitorBlockCommit(priv-mon, device,
top !topIndex ? top : topSource-path,
base !baseIndex ? base : baseSource-path,
+ NULL,
bandwidth);
qemuDomainObjExitMonitor(driver, vm);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 80d7b9d..2c9e738 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3234,13 +3234,15 @@ qemuMonitorTransaction(qemuMonitorPtr mon,
virJSONValuePtr actions)
int
qemuMonitorBlockCommit(qemuMonitorPtr mon, const char *device,
const char *top, const char *base,
+ const char *backingName,
unsigned long bandwidth)
{
int ret = -1;
unsigned long long speed;
-VIR_DEBUG(mon=%p, device=%s, top=%s, base=%s, bandwidth=%ld,
- mon, device, top, base, bandwidth);
+VIR_DEBUG(mon=%p, device=%s, top=%s, base=%s, backingName=%s,
+ bandwidth=%lu,
+ mon, device, top, base, NULLSTR(backingName), bandwidth);
/* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
* limited to LLONG_MAX also for unsigned values */
@@ -3254,7 +3256,8 @@ qemuMonitorBlockCommit(qemuMonitorPtr mon, const char
*device,
speed = 20;
if (mon-json)
-ret = qemuMonitorJSONBlockCommit(mon, device, top, base, speed);
+ret = qemuMonitorJSONBlockCommit(mon, device, top, base,
+ backingName, speed);
else
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
_(block-commit requires JSON monitor));
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 31b3204..71b7d3c 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -662,6 +662,7 @@ int qemuMonitorBlockCommit(qemuMonitorPtr mon,
const char *device,
const char *top,
const char *base,
+ const char *backingName,
unsigned long bandwidth)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
ATTRIBUTE_NONNULL(4);
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index bedd959..b4cdd6d 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3458,6 +3458,7 @@ qemuMonitorJSONTransaction(qemuMonitorPtr mon,
virJSONValuePtr actions)
int
qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char *device,
const char *top, const char *base,
+ const char *backingName,
unsigned long long speed)
{
int ret = -1;
@@ -3469,6 +3470,7 @@ qemuMonitorJSONBlockCommit(qemuMonitorPtr mon, const char
*device,
U:speed, speed,
s:top, top,
s:base, base,
+ S:backing-file, backingName,
NULL);
if (!cmd)
return -1;
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index e29158e..37ab325 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -261,6 +261,7 @@ int qemuMonitorJSONBlockCommit(qemuMonitorPtr mon,
const char *device,
const char *top,
const char *base,
+ const char *backingName,
unsigned long long bandwidth)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
ATTRIBUTE_NONNULL(4);
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index 2099dc8..9c5b837 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -1164,7 +1164,7 @@ GEN_TEST_FUNC(qemuMonitorJSONAddDevice,
some_dummy_devicestr)
GEN_TEST_FUNC(qemuMonitorJSONSetDrivePassphrase, vda, secret_passhprase)
GEN_TEST_FUNC(qemuMonitorJSONDriveMirror, vdb, /foo/bar, NULL, 1024,
[libvirt] [PATCHv4 11/21] tests: virstoragetest: Don't test relative start of backing chains
libvirt always uses an absolute path to address the top image of an
image chain. Our storage test tests also the relative path which won't
ever be used. Additionally it makes the test more complicated.
---
tests/virstoragetest.c | 79 +-
1 file changed, 20 insertions(+), 59 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index d7786bb..0e2c9ac 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -703,17 +703,12 @@ mymain(void)
#define VIR_FLATTEN_2(...) __VA_ARGS__
#define VIR_FLATTEN_1(_1) VIR_FLATTEN_2 _1
-#define TEST_CHAIN(id, relstart, absstart, format, chain1, flags1, \
- chain2, flags2, chain3, flags3, chain4, flags4) \
+#define TEST_CHAIN(id, path, format, chain1, flags1, chain2, flags2) \
do { \
-TEST_ONE_CHAIN(#id a, relstart, format, flags1,\
+TEST_ONE_CHAIN(#id a, path, format, flags1 | ABS_START,\
VIR_FLATTEN_1(chain1)); \
-TEST_ONE_CHAIN(#id b, relstart, format, flags2,\
+TEST_ONE_CHAIN(#id b, path, format, flags2 | ABS_START,\
VIR_FLATTEN_1(chain2)); \
-TEST_ONE_CHAIN(#id c, absstart, format, flags3 | ABS_START,\
- VIR_FLATTEN_1(chain3)); \
-TEST_ONE_CHAIN(#id d, absstart, format, flags4 | ABS_START,\
- VIR_FLATTEN_1(chain4)); \
} while (0)
/* The actual tests, in several groups. */
@@ -729,14 +724,10 @@ mymain(void)
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_RAW,
};
-TEST_CHAIN(1, raw, absraw, VIR_STORAGE_FILE_RAW,
- (raw), EXP_PASS,
- (raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(1, absraw, VIR_STORAGE_FILE_RAW,
(raw), EXP_PASS,
(raw), ALLOW_PROBE | EXP_PASS);
-TEST_CHAIN(2, raw, absraw, VIR_STORAGE_FILE_AUTO,
- (raw), EXP_PASS,
- (raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(2, absraw, VIR_STORAGE_FILE_AUTO,
(raw), EXP_PASS,
(raw), ALLOW_PROBE | EXP_PASS);
@@ -758,14 +749,10 @@ mymain(void)
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_RAW,
};
-TEST_CHAIN(3, qcow2, absqcow2, VIR_STORAGE_FILE_QCOW2,
- (qcow2, raw), EXP_PASS,
- (qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(3, absqcow2, VIR_STORAGE_FILE_QCOW2,
(qcow2, raw), EXP_PASS,
(qcow2, raw), ALLOW_PROBE | EXP_PASS);
-TEST_CHAIN(4, qcow2, absqcow2, VIR_STORAGE_FILE_AUTO,
- (qcow2_as_raw), EXP_PASS,
- (qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(4, absqcow2, VIR_STORAGE_FILE_AUTO,
(qcow2_as_raw), EXP_PASS,
(qcow2, raw), ALLOW_PROBE | EXP_PASS);
@@ -780,14 +767,10 @@ mymain(void)
raw.relDirRel = datadir;
/* Qcow2 file with raw as absolute backing, backing format provided */
-TEST_CHAIN(5, qcow2, absqcow2, VIR_STORAGE_FILE_QCOW2,
- (qcow2, raw), EXP_PASS,
- (qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(5, absqcow2, VIR_STORAGE_FILE_QCOW2,
(qcow2, raw), EXP_PASS,
(qcow2, raw), ALLOW_PROBE | EXP_PASS);
-TEST_CHAIN(6, qcow2, absqcow2, VIR_STORAGE_FILE_AUTO,
- (qcow2_as_raw), EXP_PASS,
- (qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(6, absqcow2, VIR_STORAGE_FILE_AUTO,
(qcow2_as_raw), EXP_PASS,
(qcow2, raw), ALLOW_PROBE | EXP_PASS);
@@ -802,9 +785,7 @@ mymain(void)
.format = VIR_STORAGE_FILE_QCOW2,
};
qcow2.relDirRel = datadir;
-TEST_CHAIN(7, wrap, abswrap, VIR_STORAGE_FILE_QCOW2,
- (wrap, qcow2, raw), EXP_PASS,
- (wrap, qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(7, abswrap, VIR_STORAGE_FILE_QCOW2,
(wrap, qcow2, raw), EXP_PASS,
(wrap, qcow2, raw), ALLOW_PROBE | EXP_PASS);
@@ -832,9 +813,7 @@ mymain(void)
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
};
-TEST_CHAIN(8, wrap, abswrap, VIR_STORAGE_FILE_QCOW2,
- (wrap_as_raw, qcow2_as_raw), EXP_PASS,
- (wrap, qcow2, raw), ALLOW_PROBE | EXP_PASS,
+TEST_CHAIN(8, abswrap, VIR_STORAGE_FILE_QCOW2,
(wrap_as_raw, qcow2_as_raw), EXP_PASS,
(wrap, qcow2, raw), ALLOW_PROBE | EXP_PASS);
@@ -849,9 +828,7 @@ mymain(void)
qcow2.relDirRel = .;
/* Qcow2 file with missing backing file but specified type */
-TEST_CHAIN(9, qcow2, absqcow2, VIR_STORAGE_FILE_QCOW2,
- (qcow2), EXP_WARN,
- (qcow2), ALLOW_PROBE | EXP_WARN,
+
[libvirt] [PATCHv4 14/21] storage: Don't store parent directory of an image explicitly
The parent directory doesn't necessarily need to be stored after we
don't mangle the path stored in the image. Remove it and tweak the code
to avoid using it.
---
src/storage/storage_driver.c | 11 ++-
src/util/virstoragefile.c| 68 ++--
src/util/virstoragefile.h| 3 --
tests/virstoragetest.c | 21 --
4 files changed, 30 insertions(+), 73 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 9ce3b62..1c8ad1e 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3082,8 +3082,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
virStorageSourcePtr backingStore = NULL;
int backingFormat;
-VIR_DEBUG(path=%s dir=%s format=%d uid=%d gid=%d probe=%d,
- src-path, NULLSTR(src-relDir), src-format,
+VIR_DEBUG(path=%s format=%d uid=%d gid=%d probe=%d,
+ src-path, src-format,
(int)uid, (int)gid, allow_probe);
/* exit if we can't load information about the current image */
@@ -3189,19 +3189,12 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
if (!(cycle = virHashCreate(5, NULL)))
return -1;
-if (!src-relDir
-!(src-relDir = mdir_name(src-path))) {
-virReportOOMError();
-goto cleanup;
-}
-
if (src-format = VIR_STORAGE_FILE_NONE)
src-format = allow_probe ? VIR_STORAGE_FILE_AUTO :
VIR_STORAGE_FILE_RAW;
ret = virStorageFileGetMetadataRecurse(src, uid, gid,
allow_probe, cycle);
- cleanup:
VIR_FREE(canonPath);
virHashFree(cycle);
return ret;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 2524efa..960d65c 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1338,9 +1338,10 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
unsigned int idx,
const char **parent)
{
+virStorageSourcePtr prev = NULL;
const char *start = chain-path;
const char *tmp;
-const char *parentDir = .;
+char *parentDir = NULL;
bool nameIsFile = virStorageIsFile(name);
size_t i;
@@ -1370,8 +1371,20 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
break;
if (nameIsFile (chain-type == VIR_STORAGE_TYPE_FILE ||
chain-type == VIR_STORAGE_TYPE_BLOCK)) {
+if (prev) {
+if (!(parentDir = mdir_name(prev-path))) {
+virReportOOMError();
+goto error;
+}
+} else {
+if (VIR_STRDUP(parentDir, .) 0)
+goto error;
+}
+
int result = virFileRelLinkPointsTo(parentDir, name,
chain-path);
+
+VIR_FREE(parentDir);
if (result 0)
goto error;
if (result 0)
@@ -1379,7 +1392,7 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
}
}
*parent = chain-path;
-parentDir = chain-relDir;
+prev = chain;
chain = chain-backingStore;
i++;
}
@@ -1518,7 +1531,6 @@ virStorageSourceClearBackingStore(virStorageSourcePtr def)
return;
VIR_FREE(def-relPath);
-VIR_FREE(def-relDir);
VIR_FREE(def-backingStoreRaw);
/* recursively free backing chain */
@@ -1577,7 +1589,6 @@
virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
const char *rel)
{
char *dirname = NULL;
-const char *parentdir = ;
virStorageSourcePtr ret;
if (VIR_ALLOC(ret) 0)
@@ -1587,23 +1598,20 @@
virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
if (VIR_STRDUP(ret-relPath, parent-backingStoreRaw) 0)
goto error;
-/* XXX Once we get rid of the need to use canonical names in path, we will
be
- * able to use mdir_name on parent-path instead of using parent-relDir */
-if (STRNEQ(parent-relDir, /))
-parentdir = parent-relDir;
-
-if (virAsprintf(ret-path, %s/%s, parentdir, rel) 0)
+if (!(dirname = mdir_name(parent-path))) {
+virReportOOMError();
goto error;
+}
-if (virStorageSourceGetActualType(parent) != VIR_STORAGE_TYPE_NETWORK) {
-ret-type = VIR_STORAGE_TYPE_FILE;
-
-/* XXX store the relative directory name for test's sake */
-if (!(ret-relDir = mdir_name(ret-path))) {
-virReportOOMError();
+if (STRNEQ(dirname, /)) {
+if (virAsprintf(ret-path, %s/%s, dirname, rel) 0)
goto error;
-}
} else {
+if (virAsprintf(ret-path, /%s, rel) 0)
+goto error;
+}
+
+if (virStorageSourceGetActualType(parent) == VIR_STORAGE_TYPE_NETWORK)
[libvirt] [PATCHv4 03/21] util: storagefile: Introduce universal function to canonicalize paths
Introduce a common function that will take a callback to resolve links
that will be used to canonicalize paths on various storage systems and
add extensive tests.
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 195 ++
src/util/virstoragefile.h | 7 ++
tests/virstoragetest.c| 108 +
4 files changed, 311 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f69cd1c..7671730 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1864,6 +1864,7 @@ virStorageGenerateQcowPassphrase;
# util/virstoragefile.h
+virStorageFileCanonicalizePath;
virStorageFileChainGetBroken;
virStorageFileChainLookup;
virStorageFileFeatureTypeFromString;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 0792dd8..ef69bf3 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -40,6 +40,7 @@
#include virutil.h
#include viruri.h
#include dirname.h
+#include virbuffer.h
#if HAVE_SYS_SYSCALL_H
# include sys/syscall.h
#endif
@@ -1928,3 +1929,197 @@ virStorageSourceNewFromBacking(virStorageSourcePtr
parent)
return ret;
}
+
+
+static char *
+virStorageFileCanonicalizeFormatPath(char **components,
+ size_t ncomponents,
+ bool beginSlash,
+ bool beginDoubleSlash)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+size_t i;
+char *ret = NULL;
+
+if (beginSlash)
+virBufferAddLit(buf, /);
+
+if (beginDoubleSlash)
+virBufferAddLit(buf, /);
+
+for (i = 0; i ncomponents; i++) {
+if (i != 0)
+virBufferAddLit(buf, /);
+
+virBufferAdd(buf, components[i], -1);
+}
+
+if (virBufferError(buf) != 0) {
+virReportOOMError();
+return NULL;
+}
+
+/* if the output string is empty just return an empty string */
+if (!(ret = virBufferContentAndReset(buf)))
+ignore_value(VIR_STRDUP(ret, ));
+
+return ret;
+}
+
+
+static int
+virStorageFileCanonicalizeInjectSymlink(const char *path,
+size_t at,
+char ***components,
+size_t *ncomponents)
+{
+char **tmp = NULL;
+char **next;
+size_t ntmp = 0;
+int ret = -1;
+
+if (!(tmp = virStringSplitCount(path, /, 0, ntmp)))
+goto cleanup;
+
+/* prepend */
+for (next = tmp; *next; next++) {
+if (VIR_INSERT_ELEMENT(*components, at, *ncomponents, *next) 0)
+goto cleanup;
+
+at++;
+}
+
+ret = 0;
+
+ cleanup:
+virStringFreeListCount(tmp, ntmp);
+return ret;
+}
+
+
+char *
+virStorageFileCanonicalizePath(const char *path,
+ virStorageFileSimplifyPathReadlinkCallback cb,
+ void *cbdata)
+{
+virHashTablePtr cycle = NULL;
+bool beginSlash = false;
+bool beginDoubleSlash = false;
+char **components = NULL;
+size_t ncomponents = 0;
+char *linkpath = NULL;
+char *currentpath = NULL;
+size_t i = 0;
+int rc;
+char *ret = NULL;
+
+if (path[0] == '/') {
+beginSlash = true;
+
+if (path[1] == '/' path[2] != '/')
+beginDoubleSlash = true;
+}
+
+if (!(cycle = virHashCreate(10, NULL)))
+goto cleanup;
+
+if (!(components = virStringSplitCount(path, /, 0, ncomponents)))
+goto cleanup;
+
+while (i ncomponents) {
+/* skip slashes and '.'s */
+if (STREQ(components[i], ) ||
+STREQ(components[i], .)) {
+VIR_FREE(components[i]);
+VIR_DELETE_ELEMENT(components, i, ncomponents);
+continue;
+}
+
+/* resolve changes to parent directory */
+if (STREQ(components[i], ..)) {
+if (!beginSlash
+(i == 0 || STREQ(components[i - 1], ..))) {
+i++;
+continue;
+}
+
+VIR_FREE(components[i]);
+VIR_DELETE_ELEMENT(components, i, ncomponents);
+
+if (i != 0) {
+VIR_FREE(components[i - 1]);
+VIR_DELETE_ELEMENT(components, i - 1, ncomponents);
+i--;
+}
+
+continue;
+}
+
+/* check if the actual path isn't resulting into a symlink */
+if (!(currentpath = virStorageFileCanonicalizeFormatPath(components,
+ i + 1,
+ beginSlash,
+
beginDoubleSlash)))
+goto cleanup;
+
+if ((rc = cb(currentpath, linkpath, cbdata)) 0)
+goto cleanup;
+
+if (rc == 0) {
+if (virHashLookup(cycle,
[libvirt] [PATCHv4 18/21] lib: Introduce flag VIR_DOMAIN_BLOCK_COMMIT_RELATIVE
Introduce flag for the block commit API to allow the commit operation to
leave the chain relatively addressed. Also adds a virsh switch to enable
this behavior.
---
include/libvirt/libvirt.h.in | 4
src/libvirt.c| 5 +
tools/virsh-domain.c | 7 +++
tools/virsh.pod | 6 --
4 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 127de11..bacdf57 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2592,6 +2592,10 @@ typedef enum {
have been committed */
VIR_DOMAIN_BLOCK_COMMIT_ACTIVE = 1 2, /* Allow a two-phase commit when
top is the active layer */
+VIR_DOMAIN_BLOCK_COMMIT_RELATIVE = 1 3, /* try to keep the backing chain
+ relative if the components
+ removed by the commit are
+ already relative */
} virDomainBlockCommitFlags;
int virDomainBlockCommit(virDomainPtr dom, const char *disk, const char *base,
diff --git a/src/libvirt.c b/src/libvirt.c
index 6c4a124..21ef41f 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -19879,6 +19879,11 @@ virDomainBlockRebase(virDomainPtr dom, const char
*disk,
* VIR_DOMAIN_BLOCK_COMMIT_DELETE, then this command will unlink all files
* that were invalidated, after the commit successfully completes.
*
+ * If @flags contains VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, the name recorded
+ * into the overlay of the @top image as path to the new backing file
+ * will be kept relative to other images in case the backing chain was
+ * using relative names.
+ *
* By default, if @base is NULL, the commit target will be the bottom of
* the backing chain; if @flags contains VIR_DOMAIN_BLOCK_COMMIT_SHALLOW,
* then the immediate backing file of @top will be used instead. If @top
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index d2bd4f2..f26a133 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1492,6 +1492,8 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
flags |= VIR_DOMAIN_BLOCK_COMMIT_SHALLOW;
if (vshCommandOptBool(cmd, delete))
flags |= VIR_DOMAIN_BLOCK_COMMIT_DELETE;
+if (vshCommandOptBool(cmd, keep-relative))
+flags |= VIR_DOMAIN_BLOCK_COMMIT_RELATIVE;
ret = virDomainBlockCommit(dom, path, base, top, bandwidth, flags);
break;
case VSH_CMD_BLOCK_JOB_COPY:
@@ -1612,6 +1614,11 @@ static const vshCmdOptDef opts_block_commit[] = {
.type = VSH_OT_BOOL,
.help = N_(with --wait, don't wait for cancel to finish)
},
+{.name = keep-relative,
+ .type = VSH_OT_BOOL,
+ .help = N_(keep the backing chain relative if it was relatively
+referenced if it was before)
+},
{.name = NULL}
};
diff --git a/tools/virsh.pod b/tools/virsh.pod
index b2fd53b..5816c0b 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -769,7 +769,7 @@ address of virtual interface (such as Idetach-interface or
Idomif-setlink) will accept the MAC address printed by this command.
=item Bblockcommit Idomain Ipath [Ibandwidth]
-{[Ibase] | [I--shallow]} [Itop] [I--delete]
+{[Ibase] | [I--shallow]} [Itop] [I--delete] [I--keep-relative]
[I--wait [I--verbose] [I--timeout Bseconds] [I--async]]
Reduce the length of a backing image chain, by committing changes at the
@@ -781,7 +781,9 @@ I--shallow can be used instead of Ibase to specify the
immediate
backing file of the resulting top image to be committed. The files
being committed are rendered invalid, possibly as soon as the operation
starts; using the I--delete flag will remove these files at the successful
-completion of the commit operation.
+completion of the commit operation. Using the I--keep-relative flag
+will try to keep the backing chain names relative (if they were
+relative before).
By default, this command returns as soon as possible, and data for
the entire disk is committed in the background; the progress of the
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 09/21] tests: virstoragetest: Remove now unused pathAbs
Separately remove the now unused variable.
---
tests/virstoragetest.c | 15 ---
1 file changed, 15 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index dd25069..11bc4dd 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -283,7 +283,6 @@ struct _testFileData
unsigned long long expCapacity;
bool expEncrypted;
const char *pathRel;
-const char *pathAbs;
const char *path;
const char *relDirRel;
const char *relDirAbs;
@@ -730,7 +729,6 @@ mymain(void)
/* Raw image, whether with right format or no specified format */
testFileData raw = {
-.pathAbs = canonraw,
.path = canonraw,
.relDirRel = .,
.relDirAbs = datadir,
@@ -749,12 +747,10 @@ mymain(void)
(raw), ALLOW_PROBE | EXP_PASS);
/* Qcow2 file with relative raw backing, format provided */
-raw.pathAbs = raw;
raw.pathRel = raw;
testFileData qcow2 = {
.expBackingStoreRaw = raw,
.expCapacity = 1024,
-.pathAbs = canonqcow2,
.path = canonqcow2,
.relDirRel = .,
.relDirAbs = datadir,
@@ -762,7 +758,6 @@ mymain(void)
.format = VIR_STORAGE_FILE_QCOW2,
};
testFileData qcow2_as_raw = {
-.pathAbs = canonqcow2,
.path = canonqcow2,
.relDirRel = .,
.relDirAbs = datadir,
@@ -788,7 +783,6 @@ mymain(void)
ret = -1;
qcow2.expBackingStoreRaw = absraw;
raw.pathRel = NULL;
-raw.pathAbs = absraw;
raw.relDirRel = datadir;
/* Qcow2 file with raw as absolute backing, backing format provided */
@@ -807,7 +801,6 @@ mymain(void)
testFileData wrap = {
.expBackingStoreRaw = absqcow2,
.expCapacity = 1024,
-.pathAbs = abswrap,
.path = canonwrap,
.relDirRel = .,
.relDirAbs = datadir,
@@ -839,7 +832,6 @@ mymain(void)
testFileData wrap_as_raw = {
.expBackingStoreRaw = absqcow2,
.expCapacity = 1024,
-.pathAbs = abswrap,
.path = canonwrap,
.relDirRel = .,
.relDirAbs = datadir,
@@ -894,7 +886,6 @@ mymain(void)
/* Qcow2 file with backing protocol instead of file */
testFileData nbd = {
-.pathAbs = nbd:example.org:6000:exportname=blah,
.path = blah,
.type = VIR_STORAGE_TYPE_NETWORK,
.format = VIR_STORAGE_FILE_RAW,
@@ -911,7 +902,6 @@ mymain(void)
testFileData qed = {
.expBackingStoreRaw = absraw,
.expCapacity = 1024,
-.pathAbs = absqed,
.path = canonqed,
.relDirRel = .,
.relDirAbs = datadir,
@@ -919,7 +909,6 @@ mymain(void)
.format = VIR_STORAGE_FILE_QED,
};
testFileData qed_as_raw = {
-.pathAbs = absqed,
.path = canonqed,
.relDirRel = .,
.relDirAbs = datadir,
@@ -934,7 +923,6 @@ mymain(void)
/* directory */
testFileData dir = {
-.pathAbs = absdir,
.path = canondir,
.relDirRel = .,
.relDirAbs = datadir,
@@ -973,7 +961,6 @@ mymain(void)
.expBackingStoreRaw = ../raw,
.expCapacity = 1024,
.pathRel = ../sub/link1,
-.pathAbs = ../sub/link1,
.path = canonqcow2,
.relDirRel = sub/../sub,
.relDirAbs = datadir /sub/../sub,
@@ -983,7 +970,6 @@ mymain(void)
testFileData link2 = {
.expBackingStoreRaw = ../sub/link1,
.expCapacity = 1024,
-.pathAbs = abslink2,
.path = canonwrap,
.relDirRel = sub,
.relDirAbs = datadir /sub,
@@ -991,7 +977,6 @@ mymain(void)
.format = VIR_STORAGE_FILE_QCOW2,
};
raw.pathRel = ../raw;
-raw.pathAbs = ../raw;
raw.relDirRel = sub/../sub/..;
raw.relDirAbs = datadir /sub/../sub/..;
TEST_CHAIN(15, sub/link2, abslink2, VIR_STORAGE_FILE_QCOW2,
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 21/21] qemu: Add support for networked disks for block pull/block rebase
Now that we are able to select images from the backing chain via indexed
access we should also convert possible network sources to
qemu-compatible strings before passing them to qemu.
---
src/qemu/qemu_driver.c | 45 +
1 file changed, 41 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2b7670e..07fd9a8 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15029,6 +15029,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
virDomainDiskDefPtr disk;
virStorageSourcePtr baseSource = NULL;
unsigned int baseIndex = 0;
+char *basePath = NULL;
+char *backingPath = NULL;
if (!virDomainObjIsActive(vm)) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
@@ -15036,6 +15038,13 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
goto cleanup;
}
+if (flags VIR_DOMAIN_BLOCK_REBASE_RELATIVE !base) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(flag VIR_DOMAIN_BLOCK_REBASE_RELATIVE is valid only
+ with non-null base ));
+goto cleanup;
+}
+
priv = vm-privateData;
if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) {
async = true;
@@ -15097,10 +15106,35 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
base, baseIndex, NULL
goto endjob;
+if (baseSource) {
+if (qemuGetDriveSourceString(baseSource, NULL, basePath) 0)
+goto endjob;
+
+if (flags VIR_DOMAIN_BLOCK_REBASE_RELATIVE) {
+if (!virQEMUCapsGet(priv-qemuCaps,
QEMU_CAPS_CHANGE_BACKING_FILE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this QEMU binary doesn't support relative
+ block pull/rebase));
+goto endjob;
+}
+
+if (virStorageFileGetRelativeBackingPath(disk-src-backingStore,
+ baseSource,
+ backingPath) 0)
+goto endjob;
+
+
+if (!backingPath) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Can't keep relative backing relationship.));
+goto endjob;
+}
+}
+}
+
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJob(priv-mon, device,
- baseIndex ? baseSource-path : base,
- NULL, bandwidth, info, mode, async);
+ret = qemuMonitorBlockJob(priv-mon, device, basePath, backingPath,
+ bandwidth, info, mode, async);
qemuDomainObjExitMonitor(driver, vm);
if (ret 0)
goto endjob;
@@ -15172,6 +15206,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
}
cleanup:
+VIR_FREE(basePath);
+VIR_FREE(backingPath);
VIR_FREE(device);
if (vm)
virObjectUnlock(vm);
@@ -15419,7 +15455,8 @@ qemuDomainBlockRebase(virDomainPtr dom, const char
*path, const char *base,
virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
VIR_DOMAIN_BLOCK_REBASE_COPY |
- VIR_DOMAIN_BLOCK_REBASE_COPY_RAW, -1);
+ VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |
+ VIR_DOMAIN_BLOCK_REBASE_RELATIVE, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
return -1;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 17/21] qemu: monitor: Add support for backing name specification for block-stream
To allow changing the name that is recorded in the top of the current
image chain used in a block pull/rebase operation, we need to specify
the backing name to qemu. This is done via the backing-file attribute
to the block-stream commad.
---
src/qemu/qemu_driver.c | 8
src/qemu/qemu_migration.c| 6 +++---
src/qemu/qemu_monitor.c | 12 +++-
src/qemu/qemu_monitor.h | 3 ++-
src/qemu/qemu_monitor_json.c | 15 +++
src/qemu/qemu_monitor_json.h | 1 +
6 files changed, 32 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 61f2beb..88d44d3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14883,7 +14883,7 @@ qemuDomainBlockPivot(virConnectPtr conn,
/* Probe the status, if needed. */
if (!disk-mirroring) {
qemuDomainObjEnterMonitor(driver, vm);
-rc = qemuMonitorBlockJob(priv-mon, device, NULL, 0, info,
+rc = qemuMonitorBlockJob(priv-mon, device, NULL, NULL, 0, info,
BLOCK_JOB_INFO, true);
qemuDomainObjExitMonitor(driver, vm);
if (rc 0)
@@ -15100,7 +15100,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorBlockJob(priv-mon, device,
baseIndex ? baseSource-path : base,
- bandwidth, info, mode, async);
+ NULL, bandwidth, info, mode, async);
qemuDomainObjExitMonitor(driver, vm);
if (ret 0)
goto endjob;
@@ -15142,8 +15142,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
virDomainBlockJobInfo dummy;
qemuDomainObjEnterMonitor(driver, vm);
-ret = qemuMonitorBlockJob(priv-mon, device, NULL, 0, dummy,
- BLOCK_JOB_INFO, async);
+ret = qemuMonitorBlockJob(priv-mon, device, NULL, NULL, 0,
+ dummy, BLOCK_JOB_INFO, async);
qemuDomainObjExitMonitor(driver, vm);
if (ret = 0)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 842f782..30175f2 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1308,7 +1308,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
_(canceled by client));
goto error;
}
-mon_ret = qemuMonitorBlockJob(priv-mon, diskAlias, NULL, 0,
+mon_ret = qemuMonitorBlockJob(priv-mon, diskAlias, NULL, NULL, 0,
info, BLOCK_JOB_INFO, true);
qemuDomainObjExitMonitor(driver, vm);
@@ -1360,7 +1360,7 @@ qemuMigrationDriveMirror(virQEMUDriverPtr driver,
continue;
if (qemuDomainObjEnterMonitorAsync(driver, vm,
QEMU_ASYNC_JOB_MIGRATION_OUT) == 0)
{
-if (qemuMonitorBlockJob(priv-mon, diskAlias, NULL, 0,
+if (qemuMonitorBlockJob(priv-mon, diskAlias, NULL, NULL, 0,
NULL, BLOCK_JOB_ABORT, true) 0) {
VIR_WARN(Unable to cancel block-job on '%s', diskAlias);
}
@@ -1426,7 +1426,7 @@ qemuMigrationCancelDriveMirror(qemuMigrationCookiePtr mig,
QEMU_ASYNC_JOB_MIGRATION_OUT) 0)
goto cleanup;
-if (qemuMonitorBlockJob(priv-mon, diskAlias, NULL, 0,
+if (qemuMonitorBlockJob(priv-mon, diskAlias, NULL, NULL, 0,
NULL, BLOCK_JOB_ABORT, true) 0)
VIR_WARN(Unable to stop block job on %s, diskAlias);
qemuDomainObjExitMonitor(driver, vm);
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 2c9e738..ed6368e 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -3354,6 +3354,7 @@ int qemuMonitorScreendump(qemuMonitorPtr mon,
int qemuMonitorBlockJob(qemuMonitorPtr mon,
const char *device,
const char *base,
+const char *backingName,
unsigned long bandwidth,
virDomainBlockJobInfoPtr info,
qemuMonitorBlockJobCmd mode,
@@ -3362,9 +3363,10 @@ int qemuMonitorBlockJob(qemuMonitorPtr mon,
int ret = -1;
unsigned long long speed;
-VIR_DEBUG(mon=%p, device=%s, base=%s, bandwidth=%luM, info=%p, mode=%o,
- modern=%d, mon, device, NULLSTR(base), bandwidth, info, mode,
- modern);
+VIR_DEBUG(mon=%p, device=%s, base=%s, backingName=%s, bandwidth=%luM,
+ info=%p, mode=%o, modern=%d,
+ mon, device, NULLSTR(base), NULLSTR(backingName),
+ bandwidth, info, mode, modern);
/* Convert bandwidth MiB to bytes - unfortunately the JSON QMP protocol is
* limited to LLONG_MAX also for
[libvirt] [PATCHv4 10/21] util: storage: Remove now redundant backingRelative from virStorageSource
Now that we store only relative names in virStorageSource's member
relPath the backingRelative member is obsolete. Remove it and adapt the
code to the removal.
---
src/util/virstoragefile.c | 4 +---
src/util/virstoragefile.h | 2 --
tests/virstoragetest.c| 8 +---
3 files changed, 2 insertions(+), 12 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index f411519..3b0a4ab 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1591,8 +1591,6 @@
virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
if (VIR_ALLOC(ret) 0)
return NULL;
-ret-backingRelative = true;
-
/* store relative name */
if (VIR_STRDUP(ret-relPath, parent-backingStoreRaw) 0)
goto error;
@@ -2161,7 +2159,7 @@ virStorageFileGetRelativeBackingPath(virStorageSourcePtr
top,
*relpath = NULL;
for (next = top; next; next = next-backingStore) {
-if (!next-backingRelative || !next-relPath) {
+if (!next-relPath) {
ret = 1;
goto cleanup;
}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 38d1720..92f30a7 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -256,8 +256,6 @@ struct _virStorageSource {
/* Name of the child backing store recorded in metadata of the
* current file. */
char *backingStoreRaw;
-/* is backing store identified as relative */
-bool backingRelative;
};
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 11bc4dd..d7786bb 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -590,13 +590,11 @@ testPathRelativePrepare(void)
else
backingchain[i].backingStore = NULL;
-backingchain[i].backingRelative = true;
+backingchain[i].relPath = NULL;
}
/* normal relative backing chain */
backingchain[0].path = (char *) /path/to/some/img;
-backingchain[0].relPath = (char *) /path/to/some/img;
-backingchain[0].backingRelative = false;
backingchain[1].path = (char *) /path/to/some/asdf;
backingchain[1].relPath = (char *) asdf;
@@ -609,8 +607,6 @@ testPathRelativePrepare(void)
/* ovirt's backing chain */
backingchain[4].path = (char *) /path/to/volume/image1;
-backingchain[4].relPath = (char *) /path/to/volume/image1;
-backingchain[4].backingRelative = false;
backingchain[5].path = (char *) /path/to/volume/image2;
backingchain[5].relPath = (char *) ../volume/image2;
@@ -623,8 +619,6 @@ testPathRelativePrepare(void)
/* some arbitrarily crazy backing chains */
backingchain[8].path = (char *) /crazy/base/image;
-backingchain[8].relPath = (char *) /crazy/base/image;
-backingchain[8].backingRelative = false;
backingchain[9].path = (char *)
/crazy/base/directory/stuff/volumes/garbage/image2;
backingchain[9].relPath = (char *)
directory/stuff/volumes/garbage/image2;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 08/21] storage: Store relative path only for relatively backed storage
Due to various refactors and compatibility with the virstoragetest the
relPath field of the virStorageSource structure was always filled either
with the relative name or the full path in case of abslutely backed
storage. Return it's original purpose to store only the relative name of
the disk if it is backed relatively and tweak the tests.
---
src/storage/storage_driver.c | 4
src/util/virstoragefile.c| 21 +
src/util/virstoragefile.h| 4 ++--
tests/virstoragetest.c | 25 +++--
4 files changed, 14 insertions(+), 40 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 4f51517..9ce3b62 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3189,10 +3189,6 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
if (!(cycle = virHashCreate(5, NULL)))
return -1;
-if (!src-relPath
-VIR_STRDUP(src-relPath, src-path) 0)
-goto cleanup;
-
if (!src-relDir
!(src-relDir = mdir_name(src-path))) {
virReportOOMError();
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index ce1fc86..f411519 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -764,11 +764,11 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr
meta,
{
int ret = -1;
-VIR_DEBUG(relPath=%s, buf=%p, len=%zu, meta-format=%d,
- meta-relPath, buf, len, meta-format);
+VIR_DEBUG(path=%s, buf=%p, len=%zu, meta-format=%d,
+ meta-path, buf, len, meta-format);
if (meta-format == VIR_STORAGE_FILE_AUTO)
-meta-format = virStorageFileProbeFormatFromBuf(meta-relPath, buf,
len);
+meta-format = virStorageFileProbeFormatFromBuf(meta-path, buf, len);
if (meta-format = VIR_STORAGE_FILE_NONE ||
meta-format = VIR_STORAGE_FILE_LAST) {
@@ -908,9 +908,6 @@ virStorageFileMetadataNew(const char *path,
ret-format = format;
ret-type = VIR_STORAGE_TYPE_FILE;
-if (VIR_STRDUP(ret-relPath, path) 0)
-goto error;
-
if (VIR_STRDUP(ret-path, path) 0)
goto error;
@@ -1376,7 +1373,8 @@ virStorageFileChainLookup(virStorageSourcePtr chain,
if (idx == i)
break;
} else {
-if (STREQ_NULLABLE(name, chain-relPath))
+if (STREQ_NULLABLE(name, chain-relPath) ||
+STREQ(name, chain-path))
break;
if (nameIsFile (chain-type == VIR_STORAGE_TYPE_FILE ||
chain-type == VIR_STORAGE_TYPE_BLOCK)) {
@@ -1595,6 +1593,10 @@
virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
ret-backingRelative = true;
+/* store relative name */
+if (VIR_STRDUP(ret-relPath, parent-backingStoreRaw) 0)
+goto error;
+
/* XXX Once we get rid of the need to use canonical names in path, we will
be
* able to use mdir_name on parent-path instead of using parent-relDir */
if (STRNEQ(parent-relDir, /))
@@ -1909,11 +1911,6 @@ virStorageSourceNewFromBacking(virStorageSourcePtr
parent)
ret = virStorageSourceNewFromBackingAbsolute(parent-backingStoreRaw);
if (ret) {
-if (VIR_STRDUP(ret-relPath, parent-backingStoreRaw) 0) {
-virStorageSourceFree(ret);
-return NULL;
-}
-
/* possibly update local type */
if (ret-type == VIR_STORAGE_TYPE_FILE) {
if (stat(ret-path, st) == 0) {
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index ff8130d..38d1720 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -247,8 +247,8 @@ struct _virStorageSource {
virStorageDriverDataPtr drv;
/* metadata about storage image which need separate fields */
-/* Name of the current file as spelled by the user (top level) or
- * metadata of the overlay (if this is a backing store). */
+/* Relative path of the backing image from the parent NULL if
+ * backed by absolute path */
char *relPath;
/* Directory to start from if backingStoreRaw is a relative file
* name. */
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 6068612..dd25069 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -116,9 +116,6 @@ testStorageFileGetMetadata(const char *path,
}
}
-if (VIR_STRDUP(ret-relPath, path) 0)
-goto error;
-
if (!(ret-relDir = mdir_name(path))) {
virReportOOMError();
goto error;
@@ -371,7 +368,6 @@ testStorageChain(const void *args)
while (elt) {
char *expect = NULL;
char *actual = NULL;
-const char *expPath;
const char *expRelDir;
if (i == data-nfiles) {
@@ -379,8 +375,6 @@ testStorageChain(const void *args)
goto cleanup;
}
-expPath = isAbs ? data-files[i]-pathAbs
-: data-files[i]-pathRel;
expRelDir =
[libvirt] [PATCHv4 04/21] storage: gluster: Add backend to return unique storage file path
Use virStorageFileSimplifyPathInternal to canonicalize gluster paths
via a callback and use it for the unique volume path retrieval API.
---
src/storage/storage_backend_gluster.c | 80 +++
1 file changed, 80 insertions(+)
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index b96d116..c8077a1 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -534,6 +534,7 @@ typedef virStorageFileBackendGlusterPriv
*virStorageFileBackendGlusterPrivPtr;
struct _virStorageFileBackendGlusterPriv {
glfs_t *vol;
+char *canonpath;
};
@@ -548,6 +549,7 @@ virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)
if (priv-vol)
glfs_fini(priv-vol);
+VIR_FREE(priv-canonpath);
VIR_FREE(priv);
src-drv-priv = NULL;
@@ -713,6 +715,80 @@ virStorageFileBackendGlusterAccess(virStorageSourcePtr src,
return glfs_access(priv-vol, src-path, mode);
}
+static int
+virStorageFileBackendGlusterReadlinkCallback(const char *path,
+ char **link,
+ void *data)
+{
+virStorageFileBackendGlusterPrivPtr priv = data;
+char *buf = NULL;
+size_t bufsiz = 0;
+ssize_t ret;
+struct stat st;
+
+*link = NULL;
+
+if (glfs_stat(priv-vol, path, st) 0) {
+virReportSystemError(errno,
+ _(failed to stat gluster path '%s'),
+ path);
+return -1;
+}
+
+if (!S_ISLNK(st.st_mode))
+return 1;
+
+ realloc:
+if (VIR_EXPAND_N(buf, bufsiz, 256) 0)
+goto error;
+
+if ((ret = glfs_readlink(priv-vol, path, buf, bufsiz)) 0) {
+virReportSystemError(errno,
+ _(failed to read link of gluster file '%s'),
+ path);
+goto error;
+}
+
+if (ret == bufsiz)
+goto realloc;
+
+buf[ret] = '\0';
+
+*link = buf;
+
+return 0;
+
+ error:
+VIR_FREE(buf);
+return -1;
+}
+
+
+static const char *
+virStorageFileBackendGlusterGetUniqueIdentifier(virStorageSourcePtr src)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+char *filePath = NULL;
+
+if (priv-canonpath)
+return priv-canonpath;
+
+if (!(filePath = virStorageFileCanonicalizePath(src-path,
+
virStorageFileBackendGlusterReadlinkCallback,
+priv)))
+return NULL;
+
+ignore_value(virAsprintf(priv-canonpath, gluster://%s:%s/%s/%s,
+ src-hosts-name,
+ src-hosts-port,
+ src-volume,
+ filePath));
+
+VIR_FREE(filePath);
+
+return priv-canonpath;
+}
+
virStorageFileBackend virStorageFileBackendGluster = {
.type = VIR_STORAGE_TYPE_NETWORK,
@@ -725,4 +801,8 @@ virStorageFileBackend virStorageFileBackendGluster = {
.storageFileStat = virStorageFileBackendGlusterStat,
.storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
.storageFileAccess = virStorageFileBackendGlusterAccess,
+
+.storageFileGetUniqueIdentifier =
virStorageFileBackendGlusterGetUniqueIdentifier,
+
+
};
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv4 12/21] tests: virstoragetest: Remove unneeded relative test plumbing
After we don't test relative paths, remove even more unnecessary cruft
from the test code.
---
tests/virstoragetest.c | 61 +++---
1 file changed, 18 insertions(+), 43 deletions(-)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 0e2c9ac..782f3a8 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -284,8 +284,7 @@ struct _testFileData
bool expEncrypted;
const char *pathRel;
const char *path;
-const char *relDirRel;
-const char *relDirAbs;
+const char *relDir;
int type;
int format;
};
@@ -295,7 +294,6 @@ enum {
EXP_FAIL = 1,
EXP_WARN = 2,
ALLOW_PROBE = 4,
-ABS_START = 8,
};
struct testChainData
@@ -328,7 +326,6 @@ testStorageChain(const void *args)
virStorageSourcePtr elt;
size_t i = 0;
char *broken = NULL;
-bool isAbs = !!(data-flags ABS_START);
meta = testStorageFileGetMetadata(data-start, data-format, -1, -1,
(data-flags ALLOW_PROBE) != 0);
@@ -367,15 +364,12 @@ testStorageChain(const void *args)
while (elt) {
char *expect = NULL;
char *actual = NULL;
-const char *expRelDir;
if (i == data-nfiles) {
fprintf(stderr, probed chain was too long\n);
goto cleanup;
}
-expRelDir = isAbs ? data-files[i]-relDirAbs
-: data-files[i]-relDirRel;
if (virAsprintf(expect,
testStorageChainFormat, i,
NULLSTR(data-files[i]-path),
@@ -383,7 +377,7 @@ testStorageChain(const void *args)
data-files[i]-expCapacity,
data-files[i]-expEncrypted,
NULLSTR(data-files[i]-pathRel),
-NULLSTR(expRelDir),
+NULLSTR(data-files[i]-relDir),
data-files[i]-type,
data-files[i]-format) 0 ||
virAsprintf(actual,
@@ -703,12 +697,10 @@ mymain(void)
#define VIR_FLATTEN_2(...) __VA_ARGS__
#define VIR_FLATTEN_1(_1) VIR_FLATTEN_2 _1
-#define TEST_CHAIN(id, path, format, chain1, flags1, chain2, flags2) \
-do { \
-TEST_ONE_CHAIN(#id a, path, format, flags1 | ABS_START,\
- VIR_FLATTEN_1(chain1)); \
-TEST_ONE_CHAIN(#id b, path, format, flags2 | ABS_START,\
- VIR_FLATTEN_1(chain2)); \
+#define TEST_CHAIN(id, path, format, chain1, flags1, chain2, flags2)
\
+do {
\
+TEST_ONE_CHAIN(#id a, path, format, flags1, VIR_FLATTEN_1(chain1));
\
+TEST_ONE_CHAIN(#id b, path, format, flags2, VIR_FLATTEN_1(chain2));
\
} while (0)
/* The actual tests, in several groups. */
@@ -719,8 +711,7 @@ mymain(void)
/* Raw image, whether with right format or no specified format */
testFileData raw = {
.path = canonraw,
-.relDirRel = .,
-.relDirAbs = datadir,
+.relDir = datadir,
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_RAW,
};
@@ -737,15 +728,13 @@ mymain(void)
.expBackingStoreRaw = raw,
.expCapacity = 1024,
.path = canonqcow2,
-.relDirRel = .,
-.relDirAbs = datadir,
+.relDir = datadir,
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
};
testFileData qcow2_as_raw = {
.path = canonqcow2,
-.relDirRel = .,
-.relDirAbs = datadir,
+.relDir = datadir,
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_RAW,
};
@@ -764,7 +753,6 @@ mymain(void)
ret = -1;
qcow2.expBackingStoreRaw = absraw;
raw.pathRel = NULL;
-raw.relDirRel = datadir;
/* Qcow2 file with raw as absolute backing, backing format provided */
TEST_CHAIN(5, absqcow2, VIR_STORAGE_FILE_QCOW2,
@@ -779,12 +767,10 @@ mymain(void)
.expBackingStoreRaw = absqcow2,
.expCapacity = 1024,
.path = canonwrap,
-.relDirRel = .,
-.relDirAbs = datadir,
+.relDir = datadir,
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
};
-qcow2.relDirRel = datadir;
TEST_CHAIN(7, abswrap, VIR_STORAGE_FILE_QCOW2,
(wrap, qcow2, raw), EXP_PASS,
(wrap, qcow2, raw), ALLOW_PROBE | EXP_PASS);
@@ -801,15 +787,13 @@ mymain(void)
-b, absqcow2, wrap, NULL);
if (virCommandRun(cmd, NULL) 0)
ret = -1;
-qcow2_as_raw.relDirRel = datadir;
/* Qcow2 file with raw as absolute backing, backing format omitted */
testFileData wrap_as_raw = {
.expBackingStoreRaw = absqcow2,
.expCapacity =
[libvirt] [PATCHv4 20/21] qemu: Add support for networked disks for block commit
Now that we are able to select images from the backing chain via indexed
access we should also convert possible network sources to
qemu-compatible strings before passing them to qemu.
---
src/qemu/qemu_driver.c | 37 +
1 file changed, 33 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 88d44d3..2b7670e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15483,9 +15483,13 @@ qemuDomainBlockCommit(virDomainPtr dom,
unsigned int baseIndex = 0;
const char *top_parent = NULL;
bool clean_access = false;
+char *topPath = NULL;
+char *basePath = NULL;
+char *backingPath = NULL;
/* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */
-virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW, -1);
+virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
+ VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
@@ -15596,6 +15600,30 @@ qemuDomainBlockCommit(virDomainPtr dom,
VIR_DISK_CHAIN_READ_WRITE) 0))
goto endjob;
+if (qemuGetDriveSourceString(topSource, NULL, topPath) 0)
+goto endjob;
+
+if (qemuGetDriveSourceString(baseSource, NULL, basePath) 0)
+goto endjob;
+
+if (flags VIR_DOMAIN_BLOCK_COMMIT_RELATIVE) {
+if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_CHANGE_BACKING_FILE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this qemu doesn't support relative blockpull));
+goto endjob;
+}
+
+if (virStorageFileGetRelativeBackingPath(topSource, baseSource,
+ backingPath) 0)
+goto endjob;
+
+if (!backingPath) {
+virReportError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Can't keep relative backing relationship.));
+goto endjob;
+}
+}
+
/* Start the commit operation. Pass the user's original spelling,
* if any, through to qemu, since qemu may behave differently
* depending on whether the input was specified as relative or
@@ -15603,9 +15631,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
* thing if the user specified a relative name). */
qemuDomainObjEnterMonitor(driver, vm);
ret = qemuMonitorBlockCommit(priv-mon, device,
- top !topIndex ? top : topSource-path,
- base !baseIndex ? base : baseSource-path,
- NULL,
+ topPath, basePath, backingPath,
bandwidth);
qemuDomainObjExitMonitor(driver, vm);
@@ -15623,6 +15649,9 @@ qemuDomainBlockCommit(virDomainPtr dom,
vm = NULL;
cleanup:
+VIR_FREE(topPath);
+VIR_FREE(basePath);
+VIR_FREE(backingPath);
VIR_FREE(device);
if (vm)
virObjectUnlock(vm);
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 07/36] storage: Switch metadata crawler to use storage driver to get unique path
On 06/09/14 16:07, Roman Bogorodskiy wrote:
Peter Krempa wrote:
On 06/07/14 20:35, Roman Bogorodskiy wrote:
Peter Krempa wrote:
Use the virStorageFileGetUniqueIdentifier() function to get a unique
identifier regardless of the target storage type instead of relying on
canonicalize_path().
A new function that checks whether we support a given image is
introduced to avoid errors for unimplemented backends.
---
Notes:
Version 3:
- fixed typo
- ACKed by Eric
src/storage/storage_driver.c | 77
+++-
1 file changed, 54 insertions(+), 23 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index f92a553..5c4188f 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3112,49 +3135,63 @@
virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
int fd;
int ret = -1;
struct stat st;
+const char *uniqueName;
virStorageSourcePtr backingStore = NULL;
int backingFormat;
-VIR_DEBUG(path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d
probe=%d,
- src-path, canonPath, NULLSTR(src-relDir), src-format,
+VIR_DEBUG(path=%s dir=%s format=%d uid=%d gid=%d probe=%d,
+ src-path, NULLSTR(src-relDir), src-format,
(int)uid, (int)gid, allow_probe);
-if (virHashLookup(cycle, canonPath)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(backing store for %s is self-referential),
- src-path);
+/* exit if we can't load information about the current image */
+if (!virStorageFileSupportsBackingChainTraversal(src))
+return 0;
After this change I get virstrogetest failing on FreeBSD, which doesn't
support any of the file storage backends currently:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 806c06400 (LWP 100061/virstoragetest)]
0x00410088 in mymain () at virstoragetest.c:937
937 TEST_LOOKUP(3, qcow2, chain-backingStore-path,
chain-backingStore,
Current language: auto; currently minimal
(gdb) p chain
$1 = 0x806c7b100
(gdb) p chain-backingStore
$2 = 0x0
(gdb)
Hmm, so FreeBSD doesn't currently compile the storage driver? We should
probably look into enabling it. All the stuff that was done by the code
was compiling just fine on FreeBSD previously so enabling just the local
filesystem storage backend should work well. I'll have a look what that
would include.
virstorage.c test calls testStorageFileGetMetadata() which calls
virStorageFileGetMetadata().
Inside of that, the call sequence is:
virStorageFileGetMetadata - virStorageFileGetMetadataRecurse
virStorageFileGetMetadataRecurse then checks
!virStorageFileSupportsBackingChainTraversal() and returns 0.
virStorageFileSupportsBackingChainTraversal tries to initialise backend
using virStorageFileBackendForTypeInternal().
virStorageFileBackendForTypeInternal loops through fileBackends which
looks this way:
static virStorageFileBackendPtr fileBackends[] = {
#if WITH_STORAGE_FS
virStorageFileBackendFile,
virStorageFileBackendBlock,
#endif
#if WITH_STORAGE_GLUSTER
virStorageFileBackendGluster,
#endif
NULL
};
FreeBSD doesn't support neither WITH_STORAGE_GLUSTER nor WITH_STORAGE_FS. So
we end up having chain-backingStore == NULL.
PS I may be wrong here as it's my first experience with the storage code
and I haven't yet spend much time on it.
Yes, the problem is that VIR_STORAGE_FS isn't enabled on freebsd.
I don't know whether it's worth analyzing the code to find out if it can
be enabled on freebsd or not. (the needed portions certainly can be
enabled, but some of the other portions might be problematic.
At any rate, I'm working on a refactor of the test that will allow to
skip the test (and remove a few ugly pieces of code)
Roman Bogorodskiy
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] build: prefer -fstack-protector-strong to -all
Try -fstack-protector-strong first on Linux. If that fails, fall back to -fstack-protector-all. --- m4/virt-compile-warnings.m4 | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 196afa7..532a777 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -164,13 +164,14 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl error: -fstack-protector not supported for this target [-Werror] ;; *-*-linux*) - dnl Fedora only uses -fstack-protector, but doesn't seem to - dnl be great overhead in adding -fstack-protector-all instead + dnl Prefer -fstack-protector-strong if it's available. + dnl There doesn't seem to be great overhead in adding + dnl -fstack-protector-all instead of -fstack-protector. dnl - dnl We also don't need ssp-buffer-size with -all, + dnl We also don't need ssp-buffer-size with -all or -strong, dnl since functions are protected regardless of buffer size. dnl wantwarn=$wantwarn --param=ssp-buffer-size=4 - wantwarn=$wantwarn -fstack-protector-all + wantwarn=$wantwarn -fstack-protector-strong ;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle @@ -201,6 +202,19 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ gl_WARN_ADD([$w]) done +case $host in +*-*-linux*) +dnl Fall back to -fstack-protector-all if -strong is not available +case $WARN_CFLAGS in +*-fstack-protector-strong*) +;; +*) +gl_WARN_ADD([-fstack-protector-all]) +;; +esac +;; +esac + # Silence certain warnings in gnulib, and use improved glibc headers AC_DEFINE([lint], [1], [Define to 1 if the compiler is checking for lint.]) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] build: prefer -fstack-protector-strong to -all
On Wed, Jun 11, 2014 at 02:11:46PM +0200, Ján Tomko wrote: Try -fstack-protector-strong first on Linux. If that fails, fall back to -fstack-protector-all. --- m4/virt-compile-warnings.m4 | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 196afa7..532a777 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -164,13 +164,14 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ dnl error: -fstack-protector not supported for this target [-Werror] ;; *-*-linux*) - dnl Fedora only uses -fstack-protector, but doesn't seem to - dnl be great overhead in adding -fstack-protector-all instead + dnl Prefer -fstack-protector-strong if it's available. + dnl There doesn't seem to be great overhead in adding + dnl -fstack-protector-all instead of -fstack-protector. dnl - dnl We also don't need ssp-buffer-size with -all, + dnl We also don't need ssp-buffer-size with -all or -strong, dnl since functions are protected regardless of buffer size. dnl wantwarn=$wantwarn --param=ssp-buffer-size=4 - wantwarn=$wantwarn -fstack-protector-all + wantwarn=$wantwarn -fstack-protector-strong ;; *-*-freebsd*) dnl FreeBSD ships old gcc 4.2.1 which doesn't handle @@ -201,6 +202,19 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ gl_WARN_ADD([$w]) done +case $host in +*-*-linux*) +dnl Fall back to -fstack-protector-all if -strong is not available +case $WARN_CFLAGS in +*-fstack-protector-strong*) +;; +*) +gl_WARN_ADD([-fstack-protector-all]) +;; +esac +;; +esac + # Silence certain warnings in gnulib, and use improved glibc headers AC_DEFINE([lint], [1], [Define to 1 if the compiler is checking for lint.]) ACK Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v8 2/4] Add vbox_snapshot_conf struct
One more here too - missed it in my last pass...
On 05/19/2014 08:47 AM, Yohan BELLEGUIC wrote:
...snip...
+
+/*
+ *isCurrentSnapshot: Return 1 if 'snapshotName' corresponds to the
+ *vboxSnapshotXmlMachinePtr's current snapshot, return 0 otherwise.
+ */
+int virVBoxSnapshotConfIsCurrentSnapshot(virVBoxSnapshotConfMachinePtr
machine, char *snapshotName)
+{
+virVBoxSnapshotConfSnapshotPtr snapshot = NULL;
+if (machine == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Machine is null));
+goto cleanup;
+}
+if (snapshotName == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(snapshotName is null));
+goto cleanup;
+}
+snapshot = virVBoxSnapshotConfSnapshotByName(machine-snapshot,
snapshotName);
Coverity complains:
(5) Event returned_null:virVBoxSnapshotConfSnapshotByName returns
null (checked 4 out of 5 times). [details]
(14) Event var_assigned:Assigning: snapshot = null return value from
virVBoxSnapshotConfSnapshotByName.
You will need a:
if (snapshot == NULL) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Unable to find the snapshot %s), snapshotName);
goto cleanup;
}
+return STREQ(snapshot-uuid, machine-currentSnapshot);
(15) Event dereference: Dereferencing a null pointer snapshot.
+
+ cleanup:
+return 0;
+}
+
+/*
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v8 3/4] vbox_tmpl.c: Patch for redefining snapshots
Another coverity error as listed in-line below in vboxSnapshotRedefine()
John
On 05/19/2014 08:47 AM, Yohan BELLEGUIC wrote:
The machine is unregistered and its vbox XML file is changed in order to
add snapshot information. The machine is then registered with the
snapshot to redefine.
---
src/vbox/vbox_tmpl.c | 976
+-
1 file changed, 970 insertions(+), 6 deletions(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index 5d4a7ba..a7f15d4 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -43,6 +43,7 @@
#include datatypes.h
#include domain_conf.h
#include snapshot_conf.h
+#include vbox_snapshot_conf.h
#include network_conf.h
#include virerror.h
#include domain_event.h
@@ -6015,6 +6016,958 @@ vboxDomainSnapshotGet(vboxGlobalData *data,
return snapshot;
}
+#if VBOX_API_VERSION = 4002000
+static int vboxCloseDisksRecursively(virDomainPtr dom, char *location)
+{
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+nsresult rc;
+size_t i = 0;
+PRUnichar *locationUtf = NULL;
+IMedium *medium = NULL;
+IMedium **children = NULL;
+PRUint32 childrenSize = 0;
+VBOX_UTF8_TO_UTF16(location, locationUtf);
+rc = data-vboxObj-vtbl-OpenMedium(data-vboxObj,
+ locationUtf,
+ DeviceType_HardDisk,
+ AccessMode_ReadWrite,
+ false,
+ medium);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unable to open HardDisk, rc=%08x),
+ (unsigned)rc);
+goto cleanup;
+}
+rc = medium-vtbl-GetChildren(medium, childrenSize, children);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s
+ , _(Unable to get disk children));
+goto cleanup;
+}
+for (i = 0; i childrenSize; i++) {
+IMedium *childMedium = children[i];
+if (childMedium) {
+PRUnichar *childLocationUtf = NULL;
+char *childLocation = NULL;
+rc = childMedium-vtbl-GetLocation(childMedium,
childLocationUtf);
+VBOX_UTF16_TO_UTF8(childLocationUtf, childLocation);
+VBOX_UTF16_FREE(childLocationUtf);
+if (vboxCloseDisksRecursively(dom, childLocation) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s
+ , _(Unable to close disk children));
+goto cleanup;
+}
+VIR_FREE(childLocation);
+}
+}
+rc = medium-vtbl-Close(medium);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unable to close HardDisk, rc=%08x),
+ (unsigned)rc);
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VBOX_UTF16_FREE(locationUtf);
+return ret;
+}
+
+static int
+vboxSnapshotRedefine(virDomainPtr dom,
+ virDomainSnapshotDefPtr def,
+ bool isCurrent)
+{
+/*
+ * If your snapshot has a parent,
+ * it will only be redefined if you have already
+ * redefined the parent.
+ *
+ * The general algorithm of this function is below :
+ * First of all, we are going to create our vboxSnapshotXmlMachinePtr
struct from
+ * the machine settings path.
+ * Then, if the machine current snapshot xml file is saved in the
machine location,
+ * it means that this snapshot was previously modified by us and has
fake disks.
+ * Fake disks are added when the flag VIR_DOMAIN_SNAPSHOT_CREATE_CURRENT
was not set
+ * yet, in order to not corrupt read-only disks. The first thing to do
is to remove those
+ * disks and restore the read-write disks, if any, in the
vboxSnapshotXmlMachinePtr struct.
+ * We also delete the current snapshot xml file.
+ *
+ * After that, we are going to register the snapshot read-only disks
that we want to redefine,
+ * if they are not in the media registry struct.
+ *
+ * The next step is to unregister the machine and close all disks.
+ *
+ * Then, we check if the flag VIR_DOMAIN_SNAPSHOT_CREATE_REDEFINE has
already been set.
+ * If this flag was set, we just add read-write disks to the media
registry
+ * struct. Otherwise, we save the snapshot xml file into the machine
location in order
+ * to recover the read-write disks during the next redefine and we
create differential disks
+ * from the snapshot read-only disks and add them to the media registry
struct.
+ *
+ * Finally, we register the machine with the new virtualbox description
file.
+ */
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+vboxIID domiid =
Re: [libvirt] [PATCH v8 4/4] vbox_tmpl.c: Add function for undefining snapshot
Another module where Coverity was less than enthusiastic about the
changes - although less issues than the snapshot code...
This one's a bit easier - add a VIR_FREE(disk); to the two places shown
below and you're good.
John
On 05/19/2014 08:47 AM, Yohan BELLEGUIC wrote:
All snapshots information will be deleted from the vbox XML, but
differencing disks will be kept so the user will be able to redefine the
snapshot.
---
src/vbox/vbox_tmpl.c | 464
+-
1 file changed, 463 insertions(+), 1 deletion(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index a7f15d4..eb577f4 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -8364,7 +8364,454 @@ vboxDomainSnapshotDeleteTree(vboxGlobalData *data,
return ret;
}
+#if VBOX_API_VERSION = 4002000
+static int
+vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
+{
+/*
+ * This function will remove the node in the vbox xml corresponding to
the snapshot.
+ * It is usually called by vboxDomainSnapshotDelete() with the flag
+ * VIR_DOMAIN_SNAPSHOT_DELETE_METADATA_ONLY.
+ * If you want to use it anywhere else, be careful, if the snapshot you
want to delete
+ * has children, the result is not granted, they will probably will be
deleted in the
+ * xml, but you may have a problem with hard drives.
+ *
+ * If the snapshot which is being deleted is the current one, we will
set the current
+ * snapshot of the machine to the parent of this snapshot. Before
writing the modified
+ * xml file, we undefine the machine from vbox. After writing the file,
we redefine
+ * the machine with the new file.
+ */
+
+virDomainPtr dom = snapshot-domain;
+VBOX_OBJECT_CHECK(dom-conn, int, -1);
+virDomainSnapshotDefPtr def= NULL;
+char *defXml = NULL;
+vboxIID domiid = VBOX_IID_INITIALIZER;
+nsresult rc;
+IMachine *machine = NULL;
+PRUnichar *settingsFilePathUtf16 = NULL;
+char *settingsFilepath = NULL;
+virVBoxSnapshotConfMachinePtr snapshotMachineDesc = NULL;
+int isCurrent = -1;
+int it = 0;
+PRUnichar *machineNameUtf16 = NULL;
+char *machineName = NULL;
+char *nameTmpUse = NULL;
+char *machineLocationPath = NULL;
+PRUint32 aMediaSize = 0;
+IMedium **aMedia = NULL;
+defXml = vboxDomainSnapshotGetXMLDesc(snapshot, 0);
+if (!defXml) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to get XML Desc of snapshot));
+goto cleanup;
+}
+def = virDomainSnapshotDefParseString(defXml,
+ data-caps,
+ data-xmlopt,
+ -1,
+ VIR_DOMAIN_SNAPSHOT_PARSE_DISKS |
+
VIR_DOMAIN_SNAPSHOT_PARSE_REDEFINE);
+if (!def) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to get a virDomainSnapshotDefPtr));
+goto cleanup;
+}
+
+vboxIIDFromUUID(domiid, dom-uuid);
+rc = VBOX_OBJECT_GET_MACHINE(domiid.value, machine);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_NO_DOMAIN, %s,
+ _(no domain with matching UUID));
+goto cleanup;
+}
+rc = machine-vtbl-GetSettingsFilePath(machine, settingsFilePathUtf16);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(cannot get settings file path));
+goto cleanup;
+}
+VBOX_UTF16_TO_UTF8(settingsFilePathUtf16, settingsFilepath);
+
+/*Getting the machine name to retrieve the machine location path.*/
+rc = machine-vtbl-GetName(machine, machineNameUtf16);
+if (NS_FAILED(rc)) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(cannot get machine name));
+goto cleanup;
+}
+VBOX_UTF16_TO_UTF8(machineNameUtf16, machineName);
+if (virAsprintf(nameTmpUse, %s.vbox, machineName) 0)
+goto cleanup;
+machineLocationPath = virStringReplace(settingsFilepath, nameTmpUse, );
+if (machineLocationPath == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to get the machine location path));
+goto cleanup;
+}
+snapshotMachineDesc = virVBoxSnapshotConfLoadVboxFile(settingsFilepath,
machineLocationPath);
+if (!snapshotMachineDesc) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(cannot create a vboxSnapshotXmlPtr));
+goto cleanup;
+}
+
+isCurrent = virVBoxSnapshotConfIsCurrentSnapshot(snapshotMachineDesc,
def-name);
+if (isCurrent 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(Unable to know if the snapshot
Re: [libvirt] [PATCH v8 2/4] Add vbox_snapshot_conf struct
Ug... and trying to locally fix the RW/RO discovered two more issues...
On 06/11/2014 08:21 AM, John Ferlan wrote:
This patch has resulted in many new Coverity errors - mostly resource
leaks as a result of the virVBoxSnapshotConfAllChildren() recursive
function. I would clean them up, but I'm a bit leary of missing some
nuance in the original design.
There were also a couple of issues in
virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML and
virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML
Details of the issues and some thoughts are inline below in the functions
These should be cleaned up before the next release...
John
...snip...
+
+/*
+ *getRWDisksPathsFromLibvirtXML: Parse a libvirt XML snapshot file,
allocates and
+ *fills a list of read-write disk paths.
+ *return array length on success, -1 on failure.
+ */
+int virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(char *filePath, char
***rwDisksPath)
+{
+int result = -1;
+size_t i = 0;
+char **ret;
Needs to be initialized to NULL (char **ret = NULL;)
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr xPathContext = NULL;
+xmlNodePtr *nodes = NULL;
+int nodeSize = 0;
+if (filePath == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(filePath is null));
+goto cleanup;
+}
+xml = virXMLParse(filePath, NULL, NULL);
+if (xml == NULL) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Unable to parse the xml));
+goto cleanup;
+}
+if (!(xPathContext = xmlXPathNewContext(xml))) {
+virReportOOMError();
+goto cleanup;
+}
+xPathContext-node = xmlDocGetRootElement(xml);
+nodeSize = virXPathNodeSet(/domainsnapshot/disks/disk, xPathContext,
nodes);
Coverity comlains that nodeSize can be a negative number. Other code in
libvirt will do something like:
if ((nodeSize = virXPathNodeSet(/domainsnapshot/disks/disk,
xPathContext, nodes)) 0)
goto cleanup;
+
+if (VIR_ALLOC_N(ret, nodeSize) 0)
+goto cleanup;
+
+for (i = 0; i nodeSize; i++) {
+xmlNodePtr node = nodes[i];
+xPathContext-node = node;
+xmlNodePtr sourceNode = virXPathNode(./source, xPathContext);
+if (sourceNode) {
+ret[i] = virXMLPropString(sourceNode, file);
+}
+}
+result = 0;
+
+ cleanup:
+xmlFreeDoc(xml);
+xmlXPathFreeContext(xPathContext);
+if (result 0) {
+virStringFreeList(ret);
+nodeSize = -1;
+}
+*rwDisksPath = ret;
+return nodeSize;
+}
+
+/*
+ *getRODisksPathsFromLibvirtXML: *Parse a libvirt XML snapshot file,
allocates and fills
+ *a list of read-only disk paths (the parents of the read-write disks).
+ *return array length on success, -1 on failure.
+ */
+int virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(char *filePath, char
***roDisksPath)
+{
+int result = -1;
+size_t i = 0;
+char **ret;
+xmlDocPtr xml = NULL;
+xmlXPathContextPtr xPathContext = NULL;
+xmlNodePtr *nodes = NULL;
+int nodeSize = 0;
+if (filePath == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(filePath is null));
+goto cleanup;
+}
+xml = virXMLParse(filePath, NULL, NULL);
+if (xml == NULL) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(Unable to parse the xml));
+goto cleanup;
+}
+if (!(xPathContext = xmlXPathNewContext(xml))) {
+virReportOOMError();
+goto cleanup;
+}
+xPathContext-node = xmlDocGetRootElement(xml);
+nodeSize = virXPathNodeSet(/domainsnapshot/domain/devices/disk,
+ xPathContext,
+ nodes);
Same issue here as the RW code - you need to handle nodeSize return:
if ((nodeSize = virXPathNodeSet(/domainsnapshot/domain/devices/disk,
xPathContext,
nodes)) 0)
goto cleanup;
+if (VIR_ALLOC_N(ret, nodeSize) 0)
+goto cleanup;
+
+for (i = 0; i nodeSize; i++) {
+xmlNodePtr node = nodes[i];
+xPathContext-node = node;
+xmlNodePtr sourceNode = virXPathNode(./source, xPathContext);
+if (sourceNode) {
+ret[i] = virXMLPropString(sourceNode, file);
+}
+}
+result = 0;
+
+ cleanup:
+xmlFreeDoc(xml);
+xmlXPathFreeContext(xPathContext);
+if (result 0) {
+virStringFreeList(ret);
+nodeSize = -1;
+}
+*roDisksPath = ret;
Use:
+} else {
+*roDisksPath = ret;
}
+return nodeSize;
+}
+
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] vbox_snapshot_conf: Resolve Coverity warnings
Clean up some Coverity warnings from commit id '4dc5d8f1'
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/vbox/vbox_snapshot_conf.c | 27 ++-
1 file changed, 18 insertions(+), 9 deletions(-)
diff --git a/src/vbox/vbox_snapshot_conf.c b/src/vbox/vbox_snapshot_conf.c
index 9c78410..676a0e1 100644
--- a/src/vbox/vbox_snapshot_conf.c
+++ b/src/vbox/vbox_snapshot_conf.c
@@ -1257,6 +1257,11 @@
virVBoxSnapshotConfIsCurrentSnapshot(virVBoxSnapshotConfMachinePtr machine,
goto cleanup;
}
snapshot = virVBoxSnapshotConfSnapshotByName(machine-snapshot,
snapshotName);
+if (snapshot == NULL) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unable to find the snapshot %s), snapshotName);
+goto cleanup;
+}
return STREQ(snapshot-uuid, machine-currentSnapshot);
cleanup:
@@ -1274,7 +1279,7 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const
char *filePath,
{
int result = -1;
size_t i = 0;
-char **ret;
+char **ret = NULL;
xmlDocPtr xml = NULL;
xmlXPathContextPtr xPathContext = NULL;
xmlNodePtr *nodes = NULL;
@@ -1296,7 +1301,9 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const
char *filePath,
goto cleanup;
}
xPathContext-node = xmlDocGetRootElement(xml);
-nodeSize = virXPathNodeSet(/domainsnapshot/disks/disk, xPathContext,
nodes);
+if ((nodeSize = virXPathNodeSet(/domainsnapshot/disks/disk,
+xPathContext, nodes)) 0)
+goto cleanup;
if (VIR_ALLOC_N(ret, nodeSize) 0)
goto cleanup;
@@ -1315,13 +1322,12 @@ virVBoxSnapshotConfGetRWDisksPathsFromLibvirtXML(const
char *filePath,
xmlFreeDoc(xml);
xmlXPathFreeContext(xPathContext);
if (result 0) {
-for (i = 0; i nodeSize; i++)
-VIR_FREE(ret[i]);
-VIR_FREE(ret);
+virStringFreeList(ret);
nodeSize = -1;
} else {
*rwDisksPath = ret;
}
+VIR_FREE(nodes);
return nodeSize;
}
@@ -1357,9 +1363,10 @@ virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const
char *filePath,
goto cleanup;
}
xPathContext-node = xmlDocGetRootElement(xml);
-nodeSize = virXPathNodeSet(/domainsnapshot/domain/devices/disk,
- xPathContext,
- nodes);
+if ((nodeSize = virXPathNodeSet(/domainsnapshot/domain/devices/disk,
+xPathContext,
+nodes)) 0)
+goto cleanup;
if (VIR_ALLOC_N(ret, nodeSize) 0)
goto cleanup;
@@ -1379,8 +1386,10 @@ virVBoxSnapshotConfGetRODisksPathsFromLibvirtXML(const
char *filePath,
if (result 0) {
virStringFreeList(ret);
nodeSize = -1;
+} else {
+*roDisksPath = ret;
}
-*roDisksPath = ret;
+VIR_FREE(nodes);
return nodeSize;
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] libxl: Resolve Coverity warnings
Resolve two Coverity issues introduced by commit id '9b8d6e1e'
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/libxl/libxl_migration.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 9fe904e..a25edf0 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -98,8 +98,7 @@ libxlDoMigrateReceive(virNetSocketPtr sock,
size_t i;
int ret;
-virNetSocketAccept(sock, client_sock);
-if (client_sock == NULL) {
+if (virNetSocketAccept(sock, client_sock) 0) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(Fail to accept migration connection));
goto cleanup;
@@ -526,8 +525,7 @@ libxlDomainMigrationFinish(virConnectPtr dconn,
cleanup:
if (event)
libxlDomainEventQueue(driver, event);
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
virObjectUnref(cfg);
return dom;
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Resolve Coverity warnings
Resolve the lower hanging fruit Coverity issues from recent commits. Still left outstanding is rework of virVBoxSnapshotConfAllChildren() code and callers. John Ferlan (3): vbox_temp: Resolve Coverity warnings vbox_snapshot_conf: Resolve Coverity warnings libxl: Resolve Coverity warnings src/libxl/libxl_migration.c | 6 ++ src/vbox/vbox_snapshot_conf.c | 27 ++- src/vbox/vbox_tmpl.c | 10 +- 3 files changed, 29 insertions(+), 14 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] vbox_temp: Resolve Coverity warnings
Clean up code to resolve Coverity RESOURCE_LEAK's from commit id's
'632b9600' and 'b739f807'.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/vbox/vbox_tmpl.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c
index da9b2b7..1ed2729 100644
--- a/src/vbox/vbox_tmpl.c
+++ b/src/vbox/vbox_tmpl.c
@@ -6267,6 +6267,11 @@ vboxSnapshotRedefine(virDomainPtr dom,
*/
parentUuid =
virVBoxSnapshotConfHardDiskUuidByLocation(snapshotMachineDesc,
realReadOnlyDisksPath[it]);
+if (parentUuid == NULL) {
+VIR_FREE(readWriteDisk);
+goto cleanup;
+}
+
if (virVBoxSnapshotConfAddHardDiskToMediaRegistry(readWriteDisk,
snapshotMachineDesc-mediaRegistry,
parentUuid) 0) {
@@ -8576,14 +8581,17 @@
vboxDomainSnapshotDeleteMetadataOnly(virDomainSnapshotPtr snapshot)
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Unable to get medium uuid, rc=%08x),
(unsigned)rc);
+VIR_FREE(disk);
goto cleanup;
}
VBOX_UTF16_TO_UTF8(uuidUtf16, uuid);
disk-uuid = uuid;
VBOX_UTF16_FREE(uuidUtf16);
-if (VIR_STRDUP(disk-location, newLocationUtf8) 0)
+if (VIR_STRDUP(disk-location, newLocationUtf8) 0) {
+VIR_FREE(disk);
goto cleanup;
+}
rc = newMedium-vtbl-GetFormat(newMedium, formatUtf16);
VBOX_UTF16_TO_UTF8(formatUtf16, format);
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4] qemu: Properly label FDs when restoring domain with static label
When saving domain with relabel=no, the file that gets created must have the context set anyway. That way restore can be successful without the need of relabeling the file. Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com --- src/qemu/qemu_driver.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5d40239..cf5c27c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2966,6 +2966,9 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, if (fd 0) goto cleanup; +if (virSecurityManagerSetImageFDLabel(driver-securityManager, vm-def, fd) 0) +goto cleanup; + if (!(wrapperFd = virFileWrapperFdNew(fd, path, wrapperFlags))) goto cleanup; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] Add unique_id to nodedev output
On 06/11/2014 05:31 AM, Daniel P. Berrange wrote:
On Tue, Jun 10, 2014 at 03:03:55PM -0400, John Ferlan wrote:
Add an optional unique_id parameter to nodedev. Allows for easier lookup
and display of the unique_id value in order to document for use with
scsi_host code.
Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatnode.html.in| 11 +++
docs/schemas/nodedev.rng | 6 ++
src/conf/node_device_conf.c| 23
--
src/conf/node_device_conf.h| 1 +
src/node_device/node_device_linux_sysfs.c | 6 ++
src/test/test_driver.c | 5 +++--
.../pci_8086_27c5_scsi_host_0_unique_id.xml| 8
tests/nodedevxml2xmltest.c | 1 +
8 files changed, 53 insertions(+), 8 deletions(-)
create mode 100644
tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 76bf8af..cdf0277 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -173,6 +173,17 @@
dl
dtcodehost/code/dt
ddThe SCSI host number./dd
+ dtcodeunique_id/code/dt
+ ddOn input, this optionally provides the value from the
+'unique_id' file found in the scsi_host's directory. To
+view the values of all 'unique_id' files, use codefind -H
+/sys/class/scsi_host/host{0..9}/unique_id |
+xargs grep '[0-9]'/code. On output, if the unique_id
+file exists, the value from the file will be displayed.
+This can be used in order to help uniquely identify the
+scsi_host adapter in a a href=formatstorage.html
+Storage Pool/a. span class=sinceSince 1.2.6/span
+ /dd
Where is the data from the 'unique_id' coming from ? Is this since that
Linux makes up, or is it some standard attribute from the hardware ? It
would be desirable to document what it is in a way that's not simply
refering to Linux sysfs, since sysfs itself is mostly undocumented :-)
Using 'unique_id' was from Osier's initial design - I 'assumed' that his
research into this had determined using this was unique enough.
A small amount of searching turns this up:
https://lkml.org/lkml/2011/8/17/274
...
+What: /sys/bus/scsi/devices/host*/scsi_host/host*/unique_id
+Date: February, 2003
+KernelVersion: Unknown
+Contact: James Bottomley james.bottom...@hansenpartnership.com
+Description:
+ Read only unsigned integer uniquely identifying the SCSI host
+ within the system. This value is assigned by the low level
+ driver.
+
...
Also from various sources, there's '/linux/include/scsi/scsi_host.h
/*
* This is a unique identifier that must be assigned so that we
* have some way of identifying each detected host adapter properly
* and uniquely. For hosts that do not support more than one card
* in the system at one time, this does not need to be set. It is
* initialized to 0 in scsi_register.
*/
unsigned int unique_id;
Regards,
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] Add unique_id to nodedev output
On Wed, Jun 11, 2014 at 10:00:24AM -0400, John Ferlan wrote:
On 06/11/2014 05:31 AM, Daniel P. Berrange wrote:
On Tue, Jun 10, 2014 at 03:03:55PM -0400, John Ferlan wrote:
Add an optional unique_id parameter to nodedev. Allows for easier lookup
and display of the unique_id value in order to document for use with
scsi_host code.
Signed-off-by: John Ferlan jfer...@redhat.com
---
docs/formatnode.html.in| 11 +++
docs/schemas/nodedev.rng | 6 ++
src/conf/node_device_conf.c| 23
--
src/conf/node_device_conf.h| 1 +
src/node_device/node_device_linux_sysfs.c | 6 ++
src/test/test_driver.c | 5 +++--
.../pci_8086_27c5_scsi_host_0_unique_id.xml| 8
tests/nodedevxml2xmltest.c | 1 +
8 files changed, 53 insertions(+), 8 deletions(-)
create mode 100644
tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml
diff --git a/docs/formatnode.html.in b/docs/formatnode.html.in
index 76bf8af..cdf0277 100644
--- a/docs/formatnode.html.in
+++ b/docs/formatnode.html.in
@@ -173,6 +173,17 @@
dl
dtcodehost/code/dt
ddThe SCSI host number./dd
+ dtcodeunique_id/code/dt
+ ddOn input, this optionally provides the value from the
+'unique_id' file found in the scsi_host's directory. To
+view the values of all 'unique_id' files, use codefind
-H
+/sys/class/scsi_host/host{0..9}/unique_id |
+xargs grep '[0-9]'/code. On output, if the unique_id
+file exists, the value from the file will be displayed.
+This can be used in order to help uniquely identify the
+scsi_host adapter in a a href=formatstorage.html
+Storage Pool/a. span class=sinceSince 1.2.6/span
+ /dd
Where is the data from the 'unique_id' coming from ? Is this since that
Linux makes up, or is it some standard attribute from the hardware ? It
would be desirable to document what it is in a way that's not simply
refering to Linux sysfs, since sysfs itself is mostly undocumented :-)
Using 'unique_id' was from Osier's initial design - I 'assumed' that his
research into this had determined using this was unique enough.
A small amount of searching turns this up:
https://lkml.org/lkml/2011/8/17/274
...
+What:/sys/bus/scsi/devices/host*/scsi_host/host*/unique_id
+Date:February, 2003
+KernelVersion: Unknown
+Contact: James Bottomley james.bottom...@hansenpartnership.com
+Description:
+ Read only unsigned integer uniquely identifying the SCSI host
+ within the system. This value is assigned by the low level
+ driver.
+
...
Also from various sources, there's '/linux/include/scsi/scsi_host.h
/*
* This is a unique identifier that must be assigned so that we
* have some way of identifying each detected host adapter properly
* and uniquely. For hosts that do not support more than one card
* in the system at one time, this does not need to be set. It is
* initialized to 0 in scsi_register.
*/
unsigned int unique_id;
Hmm, that's all rather fuzzy. I wonder how much better this is unique
int ID is than the current hostNN numbers. eg is this unique id stable
when you plug / unplug iSCSI targets in arbitrary order
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4] qemu: Properly label FDs when restoring domain with static label
On Wed, Jun 11, 2014 at 09:48:34AM -0400, Shivaprasad G Bhat wrote: When saving domain with relabel=no, the file that gets created must have the context set anyway. That way restore can be successful without the need of relabeling the file. Signed-off-by: Shivaprasad G Bhat sb...@linux.vnet.ibm.com --- src/qemu/qemu_driver.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5d40239..cf5c27c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2966,6 +2966,9 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver, if (fd 0) goto cleanup; +if (virSecurityManagerSetImageFDLabel(driver-securityManager, vm-def, fd) 0) +goto cleanup; + if (!(wrapperFd = virFileWrapperFdNew(fd, path, wrapperFlags))) goto cleanup; ACK Pushed. Thanks, Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: exempt graphic binaries from syntax check
On 06/11/2014 02:56 AM, Martin Kletzander wrote: On Tue, Jun 10, 2014 at 01:39:03PM -0600, Eric Blake wrote: Roman Bogorodskiy reported a syntax-check failure when using FreeBSD; complaining that: prohibit_empty_first_line tools/libvirt_win_icon_16x16.ico:1: tools/libvirt_win_icon_32x32.ico:1: tools/libvirt_win_icon_48x48.ico:1: tools/libvirt_win_icon_64x64.ico:1: maint.mk: Prohibited empty first line In reality, the first 'line' of that file is NOT empty; but since it is a binary file, awk is not required to handle it gracefully. The simplest solution is to exempt all image files from syntax checks in the first place - after all, we only store them in git because they are inconvenient to regenerate, but they are not our preferred format for making modifications, and syntax check should only cover files that we are likely to modify. * cfg.mk (VC_LIST_ALWAYS_EXCLUDE_REGEX): Exempt images. (exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF): Simplify. (exclude_file_name_regexp--sc_trailing_blank): Likewise. ACK Pushed. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 6/8] Add unique_id to nodedev output
On 06/11/2014 10:10 AM, Daniel P. Berrange wrote: On Wed, Jun 11, 2014 at 10:00:24AM -0400, John Ferlan wrote: On 06/11/2014 05:31 AM, Daniel P. Berrange wrote: On Tue, Jun 10, 2014 at 03:03:55PM -0400, John Ferlan wrote: Where is the data from the 'unique_id' coming from ? Is this since that Linux makes up, or is it some standard attribute from the hardware ? It would be desirable to document what it is in a way that's not simply refering to Linux sysfs, since sysfs itself is mostly undocumented :-) Using 'unique_id' was from Osier's initial design - I 'assumed' that his research into this had determined using this was unique enough. A small amount of searching turns this up: https://lkml.org/lkml/2011/8/17/274 ... +What: /sys/bus/scsi/devices/host*/scsi_host/host*/unique_id +Date: February, 2003 +KernelVersion: Unknown +Contact:James Bottomley james.bottom...@hansenpartnership.com +Description: +Read only unsigned integer uniquely identifying the SCSI host +within the system. This value is assigned by the low level +driver. + ... Also from various sources, there's '/linux/include/scsi/scsi_host.h /* * This is a unique identifier that must be assigned so that we * have some way of identifying each detected host adapter properly * and uniquely. For hosts that do not support more than one card * in the system at one time, this does not need to be set. It is * initialized to 0 in scsi_register. */ unsigned int unique_id; Hmm, that's all rather fuzzy. I wonder how much better this is unique int ID is than the current hostNN numbers. eg is this unique id stable when you plug / unplug iSCSI targets in arbitrary order Not sure I can answer with authority on the plug/unplug topic; however, as I understand it the hostNN numbers can change what they are pointing at after perhaps a reboot or scsi kernel module reload. I'm not an expert in these matters, but as I do understand it hardware is scanned and devices named in the order they are found. Using the PCI address and unique_id in combination provides a bit more of a chance that if the hostNN number changes, then libvirt will/could find the same adapter that was previously used. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] libxl: Resolve Coverity warnings
John Ferlan wrote:
Resolve two Coverity issues introduced by commit id '9b8d6e1e'
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/libxl/libxl_migration.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/libxl/libxl_migration.c b/src/libxl/libxl_migration.c
index 9fe904e..a25edf0 100644
--- a/src/libxl/libxl_migration.c
+++ b/src/libxl/libxl_migration.c
@@ -98,8 +98,7 @@ libxlDoMigrateReceive(virNetSocketPtr sock,
size_t i;
int ret;
-virNetSocketAccept(sock, client_sock);
-if (client_sock == NULL) {
+if (virNetSocketAccept(sock, client_sock) 0) {
virReportError(VIR_ERR_OPERATION_INVALID, %s,
_(Fail to accept migration connection));
goto cleanup;
@@ -526,8 +525,7 @@ libxlDomainMigrationFinish(virConnectPtr dconn,
cleanup:
if (event)
libxlDomainEventQueue(driver, event);
-if (vm)
-virObjectUnlock(vm);
+virObjectUnlock(vm);
virObjectUnref(cfg);
return dom;
}
ACK.
Regards,
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V8 2/2] libxl: add migration support
John Ferlan wrote:
This patch has resulted in a new Coverity warnings (looking at them was
just lower on my list of things to do lately)...
Anyway - see libxlDoMigrateReceive() and libxlDomainMigrationFinish()
for the details...
Should have looked at this before responding to your patch to fix the
warnings. Does that patch fix all the issues noted below?
Regards,
Jim
John
On 06/04/2014 04:35 PM, Jim Fehlig wrote:
This patch adds initial migration support to the libxl driver,
using the VIR_DRV_FEATURE_MIGRATION_PARAMS family of migration
functions.
Signed-off-by: Jim Fehlig jfeh...@suse.com
---
po/POTFILES.in | 1 +
src/Makefile.am | 3 +-
src/libxl/libxl_conf.h | 6 +
src/libxl/libxl_domain.h| 1 +
src/libxl/libxl_driver.c| 235 ++
src/libxl/libxl_migration.c | 585
src/libxl/libxl_migration.h | 79 ++
7 files changed, 909 insertions(+), 1 deletion(-)
diff --git a/po/POTFILES.in b/po/POTFILES.in
index cff92d9..2ee7225 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -74,6 +74,7 @@ src/lxc/lxc_process.c
src/libxl/libxl_domain.c
src/libxl/libxl_driver.c
src/libxl/libxl_conf.c
+src/libxl/libxl_migration.c
src/network/bridge_driver.c
src/network/bridge_driver_linux.c
src/network/leaseshelper.c
diff --git a/src/Makefile.am b/src/Makefile.am
index d82ca26..01af164 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -707,7 +707,8 @@ XENAPI_DRIVER_SOURCES =
\
LIBXL_DRIVER_SOURCES = \
libxl/libxl_conf.c libxl/libxl_conf.h \
libxl/libxl_domain.c libxl/libxl_domain.h \
-libxl/libxl_driver.c libxl/libxl_driver.h
+libxl/libxl_driver.c libxl/libxl_driver.h \
+libxl/libxl_migration.c libxl/libxl_migration.h
UML_DRIVER_SOURCES =\
uml/uml_conf.c uml/uml_conf.h \
diff --git a/src/libxl/libxl_conf.h b/src/libxl/libxl_conf.h
index 433d6da..6aa36d2 100644
--- a/src/libxl/libxl_conf.h
+++ b/src/libxl/libxl_conf.h
@@ -43,6 +43,9 @@
# define LIBXL_VNC_PORT_MIN 5900
# define LIBXL_VNC_PORT_MAX 65535
+# define LIBXL_MIGRATION_PORT_MIN 49152
+# define LIBXL_MIGRATION_PORT_MAX 49216
+
# define LIBXL_CONFIG_DIR SYSCONFDIR /libvirt/libxl
# define LIBXL_AUTOSTART_DIR LIBXL_CONFIG_DIR /autostart
# define LIBXL_STATE_DIR LOCALSTATEDIR /run/libvirt/libxl
@@ -115,6 +118,9 @@ struct _libxlDriverPrivate {
/* Immutable pointer, self-locking APIs */
virPortAllocatorPtr reservedVNCPorts;
+/* Immutable pointer, self-locking APIs */
+virPortAllocatorPtr migrationPorts;
+
/* Immutable pointer, lockless APIs*/
virSysinfoDefPtr hostsysinfo;
};
diff --git a/src/libxl/libxl_domain.h b/src/libxl/libxl_domain.h
index 6939008..f459fdf 100644
--- a/src/libxl/libxl_domain.h
+++ b/src/libxl/libxl_domain.h
@@ -69,6 +69,7 @@ struct _libxlDomainObjPrivate {
virChrdevsPtr devs;
libxl_evgen_domain_death *deathW;
libxlDriverPrivatePtr driver;
+unsigned short migrationPort;
struct libxlDomainJobObj job;
};
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 515d5c9..9feacb1 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -45,6 +45,7 @@
#include libxl_domain.h
#include libxl_driver.h
#include libxl_conf.h
+#include libxl_migration.h
#include xen_xm.h
#include xen_sxpr.h
#include virtypedparam.h
@@ -209,6 +210,7 @@ libxlStateCleanup(void)
virObjectUnref(libxl_driver-xmlopt);
virObjectUnref(libxl_driver-domains);
virObjectUnref(libxl_driver-reservedVNCPorts);
+virObjectUnref(libxl_driver-migrationPorts);
virObjectEventStateFree(libxl_driver-domainEventState);
virSysinfoDefFree(libxl_driver-hostsysinfo);
@@ -301,6 +303,13 @@ libxlStateInitialize(bool privileged,
LIBXL_VNC_PORT_MAX)))
goto error;
+/* Allocate bitmap for migration port reservation */
+if (!(libxl_driver-migrationPorts =
+ virPortAllocatorNew(_(migration),
+ LIBXL_MIGRATION_PORT_MIN,
+ LIBXL_MIGRATION_PORT_MAX)))
+goto error;
+
if (!(libxl_driver-domains = virDomainObjListNew()))
goto error;
@@ -4153,6 +4162,7 @@ libxlConnectSupportsFeature(virConnectPtr conn, int
feature)
switch (feature) {
case VIR_DRV_FEATURE_TYPED_PARAM_STRING:
+case VIR_DRV_FEATURE_MIGRATION_PARAMS:
return 1;
default:
return 0;
@@ -4331,6 +4341,226 @@ libxlNodeDeviceReset(virNodeDevicePtr dev)
return ret;
}
+static char *
+libxlDomainMigrateBegin3Params(virDomainPtr domain,
+
Re: [libvirt] [PATCH v3 2/2] maint: prohibit empty first lines
On 06/11/2014 12:54 AM, Martin Kletzander wrote: It started to fail for me with this change: prohibit_empty_first_line tools/libvirt_win_icon_16x16.ico:1: Hmm, it might be simpler just to globally exclude all image formats, rather than repeating lists of image files in affected rules. Definitely. It's weird, though, that I'm not seeing this problem. Is it because I'm building on Linux? Why didn't some test box we have fail as well? Correct - Linux wasn't seeing the failure, because on Linux, 'awk' is GNU awk which gracefully handles embedded NUL bytes. On FreeBSD, 'awk' is a different implementation, which chokes on binary files; and the resulting choke triggered a false positive in the syntax check. but my thoughts are that we should really ditch those one-off exception lists for graphic binary files, and instead modify this list: VC_LIST_ALWAYS_EXCLUDE_REGEX = \ (^(HACKING|docs/(news\.html\.in|.*\.patch))|\.po)$$ And this looks like the right way to go. It would deal with possible future problems as well. After figuring out the parentheses, I can ACK this to go in if you want. I'm hoping to get to cleaning up the excludes one day. It's in, now: https://www.redhat.com/archives/libvir-list/2014-June/msg00473.html -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] storage: volume: Rework lookup of volume objects
On 06/05/2014 01:52 PM, Peter Krempa wrote:
Add a helper to do all the lookup steps and remove a ton of duplicated
code.
---
src/storage/storage_driver.c | 292
++-
1 file changed, 69 insertions(+), 223 deletions(-)
ACK
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index c9916ff..26b2601 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1520,45 +1520,75 @@ storageVolDeleteInternal(virStorageVolPtr obj,
}
-static int
-storageVolDelete(virStorageVolPtr obj,
- unsigned int flags)
+static virStorageVolDefPtr
+virStorageVolDefFromVol(virStorageVolPtr obj,
+virStoragePoolObjPtr *pool,
+virStorageBackendPtr *backend)
{
virStorageDriverStatePtr driver = obj-conn-storagePrivateData;
-virStoragePoolObjPtr pool;
-virStorageBackendPtr backend;
virStorageVolDefPtr vol = NULL;
-int ret = -1;
+
+*pool = NULL;
+if (backend)
+*backend = NULL;
Initializing *pool here might be useful if someone decides to rearrange the
code again, since it's used in the cleanup section, but I don't think we need
to initialize backend - the caller should only use the values if this
function returns non-NULL.
storageDriverLock(driver);
-pool = virStoragePoolObjFindByName(driver-pools, obj-pool);
+*pool = virStoragePoolObjFindByName(driver-pools, obj-pool);
storageDriverUnlock(driver);
-if (!pool) {
+if (!*pool) {
virReportError(VIR_ERR_NO_STORAGE_POOL,
_(no storage pool with matching name '%s'),
obj-pool);
-goto cleanup;
+return NULL;
}
+
+
+static int
+storageVolDelete(virStorageVolPtr obj,
+ unsigned int flags)
+{
+virStoragePoolObjPtr pool;
+virStorageBackendPtr backend;
+virStorageVolDefPtr vol = NULL;
+int ret = -1;
+
+if (!(vol = virStorageVolDefFromVol(obj, pool, backend)))
+goto cleanup;
You can return -1 here.
+
if (virStorageVolDeleteEnsureACL(obj-conn, pool-def, vol) 0)
goto cleanup;
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] storage: pool: Fix handling of errors on pool lookup failure
On 06/05/2014 01:52 PM, Peter Krempa wrote: Rework internal pool lookup code to avoid printing the raw UUID buffer in the case a storage pool can't be found: $ virsh pool-name e012ace0-0460-5810-39ef-1bce5fa5a4dd error: failed to get pool 'e012ace0-0460-5810-39ef-1bce5fa5a4dd' error: Storage pool not found: no storage pool with matching uuid à¬à`X9ï_¥¤Ý The rework is mostly done by switching the lookup code to the newly introduced helper virStoragePoolObjFromStoragePoo *Pool Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1104993 --- src/storage/storage_driver.c | 263 +++ 1 file changed, 90 insertions(+), 173 deletions(-) ACK diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 4f51517..c9916ff 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -287,8 +289,7 @@ storagePoolLookupByUUID(virConnectPtr conn, NULL, NULL); cleanup: -if (pool) -virStoragePoolObjUnlock(pool); +virStoragePoolObjUnlock(pool); But IMO it would look neater with these no-op changes separated. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] leaseshelper: fix crash
Commit baafe668 introduced new leaseshelper with a crash of freeing env string. Calling 'getenv()' inside 'virGetEnvAllowSUID()' may return a static string and we definitely should not free it. The author probably want to free the copy of that string. Signed-off-by: Pavel Hrdina phrd...@redhat.com --- pushed under trivial rule src/network/leaseshelper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/network/leaseshelper.c b/src/network/leaseshelper.c index b6f6c32..69081c3 100644 --- a/src/network/leaseshelper.c +++ b/src/network/leaseshelper.c @@ -351,7 +351,7 @@ main(int argc, char **argv) virPidFileReleasePath(pid_file, pid_file_fd); VIR_FREE(pid_file); -VIR_FREE(exptime_tmp); +VIR_FREE(exptime); VIR_FREE(custom_lease_file); virJSONValueFree(lease_new); virJSONValueFree(leases_array); -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] virnetdev: Introduce virNetDevGetLinkInfo
On 11.06.2014 11:19, Michal Privoznik wrote: snip/ Seems we should be able to set up a dummy /sys/class/net environment in order to test the new API's... Something I attempted with my recently posted scsi_host changes. Yeah, unit testing is certainly in place. But honestly, I thought it would blow the size of this patches up. Again, something that a follow up patch will do. So, now that I'm thinking about how to implement this I see two possible ways which I'm unable to choose between: 1) mock higher level functions like virNetDevSysfsFile() or virNetDevSetupControl(). So if a function wants to reach say /sys/class/net/eth0/operstate we can direct it into tests/virnetdevdata/class/net/eth0/operstate Pros: - small set of high level functions, so the code should be rather small. - we can keep whole sysfs dir struct in git Cons: - Unable to simulate some corner cases (e.g. EINVAL on read()). - keeping whole sysfs dir struct in git 2) mock lowlevel functions, like open(), ioctl(), etc. There's plenty of functions in virnetdev.c that use lowlevel functions like socket(), ioctl() to get data. Pros: - we can model kernel behavior more closely (e.g. even run time setting of some values like MAC/IP addressese, link state, you name it) - clean git Cons: - much more code I'm not hesitant to write the code, but I'd rather have an agreement on the design before that. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] virnetdev: Introduce virNetDevGetLinkInfo
On Wed, Jun 11, 2014 at 05:41:11PM +0200, Michal Privoznik wrote: On 11.06.2014 11:19, Michal Privoznik wrote: snip/ Seems we should be able to set up a dummy /sys/class/net environment in order to test the new API's... Something I attempted with my recently posted scsi_host changes. Yeah, unit testing is certainly in place. But honestly, I thought it would blow the size of this patches up. Again, something that a follow up patch will do. So, now that I'm thinking about how to implement this I see two possible ways which I'm unable to choose between: 1) mock higher level functions like virNetDevSysfsFile() or virNetDevSetupControl(). So if a function wants to reach say /sys/class/net/eth0/operstate we can direct it into tests/virnetdevdata/class/net/eth0/operstate Pros: - small set of high level functions, so the code should be rather small. - we can keep whole sysfs dir struct in git Cons: - Unable to simulate some corner cases (e.g. EINVAL on read()). - keeping whole sysfs dir struct in git 2) mock lowlevel functions, like open(), ioctl(), etc. There's plenty of functions in virnetdev.c that use lowlevel functions like socket(), ioctl() to get data. Pros: - we can model kernel behavior more closely (e.g. even run time setting of some values like MAC/IP addressese, link state, you name it) - clean git Cons: - much more code I'm not hesitant to write the code, but I'd rather have an agreement on the design before that. To me the answer depends on what you're trying to test. If you're trying to test that functions in virnetdev.c do the right thing with syscalls/files they use, then mocking the lowlevel is best. If you're trying to test code that merely uses functions in virnetdev.c then it is valid to mock the virnetdev functions themselves. If you're trying to do both, then mocking the lowlevel functions will cope with both. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/5] virsh: improve blockcopy UI
Peter's review of my addition of active block commit pointed out
some issues that I had copied from block copy. It makes sense to
allow the shorter command-line of 'blockcopy $dom $disk --pivot'
without having to explicitly specify --wait. And my use of
embedded ?: ternaries bordered on unreadable.
* tools/virsh-domain.c (cmdBlockCopy): Make --pivot and --finish
imply --wait. Drop excess ?: operators.
* tools/virsh.pod (blockcopy): Update documentation.
Signed-off-by: Eric Blake ebl...@redhat.com
---
tools/virsh-domain.c | 25 ++---
tools/virsh.pod | 17 +
2 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index e9162db..294b594 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1786,15 +1786,15 @@ static const vshCmdOptDef opts_block_copy[] = {
},
{.name = timeout,
.type = VSH_OT_INT,
- .help = N_(with --wait, abort if copy exceeds timeout (in seconds))
+ .help = N_(implies --wait, abort if copy exceeds timeout (in seconds))
},
{.name = pivot,
.type = VSH_OT_BOOL,
- .help = N_(with --wait, pivot when mirroring starts)
+ .help = N_(implies --wait, pivot when mirroring starts)
},
{.name = finish,
.type = VSH_OT_BOOL,
- .help = N_(with --wait, quit when mirroring starts)
+ .help = N_(implies --wait, quit when mirroring starts)
},
{.name = async,
.type = VSH_OT_BOOL,
@@ -1808,10 +1808,10 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
bool ret = false;
-bool blocking = vshCommandOptBool(cmd, wait);
bool verbose = vshCommandOptBool(cmd, verbose);
bool pivot = vshCommandOptBool(cmd, pivot);
bool finish = vshCommandOptBool(cmd, finish);
+bool blocking = vshCommandOptBool(cmd, wait);
int timeout = 0;
struct sigaction sig_action;
struct sigaction old_sig_action;
@@ -1822,6 +1822,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
bool quit = false;
int abort_flags = 0;
+blocking |= vshCommandOptBool(cmd, timeout) || pivot || finish;
if (blocking) {
if (pivot finish) {
vshError(ctl, %s, _(cannot mix --pivot and --finish));
@@ -1844,8 +1845,7 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
sigaction(SIGINT, sig_action, old_sig_action);
GETTIMEOFDAY(start);
-} else if (verbose || vshCommandOptBool(cmd, timeout) ||
- vshCommandOptBool(cmd, async) || pivot || finish) {
+} else if (verbose || vshCommandOptBool(cmd, async)) {
vshError(ctl, %s, _(missing --wait option));
return false;
}
@@ -1911,11 +1911,14 @@ cmdBlockCopy(vshControl *ctl, const vshCmd *cmd)
vshError(ctl, _(failed to finish job for disk %s), path);
goto cleanup;
}
-vshPrint(ctl, \n%s,
- quit ? _(Copy aborted) :
- pivot ? _(Successfully pivoted) :
- finish ? _(Successfully copied) :
- _(Now in mirroring phase));
+if (quit)
+vshPrint(ctl, \n%s, _(Copy aborted));
+else if (pivot)
+vshPrint(ctl, \n%s, _(Successfully pivoted));
+else if (finish)
+vshPrint(ctl, \n%s, _(Successfully copied));
+else
+vshPrint(ctl, \n%s, _(Now in mirroring phase));
ret = true;
cleanup:
diff --git a/tools/virsh.pod b/tools/virsh.pod
index 84a60a7..77013f5 100644
--- a/tools/virsh.pod
+++ b/tools/virsh.pod
@@ -804,8 +804,8 @@ Ibandwidth specifies copying bandwidth limit in MiB/s,
although for
qemu, it may be non-zero only for an online domain.
=item Bblockcopy Idomain Ipath Idest [Ibandwidth] [I--shallow]
-[I--reuse-external] [I--raw] [I--wait [I--verbose]
-[{I--pivot | I--finish}] [I--timeout Bseconds] [I--async]]
+[I--reuse-external] [I--raw] [I--wait [I--async] [I--verbose]]
+[{I--pivot | I--finish}] [I--timeout Bseconds]
Copy a disk backing image chain to Idest. By default, this command
flattens the entire chain; but if I--shallow is specified, the copy
@@ -834,12 +834,13 @@ faithful copy of that point in time. However, if
I--wait is specified,
then this command will block until the mirroring phase begins, or cancel
the operation if the optional Itimeout in seconds elapses or SIGINT is
sent (usually with CCtrl-C). Using I--verbose along with I--wait
-will produce periodic status updates. Using I--pivot or I--finish
-along with I--wait will additionally end the job cleanly rather than
-leaving things in the mirroring phase. If job cancellation is triggered,
-I--async will return control to the user as fast as possible, otherwise
-the command may continue to block a little while longer until the job
-is done cleaning up.
+will produce periodic status updates. Using I--pivot (similar to
+Bblockjob I--pivot) or I--finish (similar to Bblockjob I--abort)
+implies I--wait, and will additionally end the job cleanly rather than
+leaving things in the
[libvirt] [PATCH v3 2/5] virsh: expose new active commit controls
Add knobs to virsh to manage a 2-phase active commit of the top
layer, similar to knobs already present on blockcopy. While this
code will fail until later patches actually implement the new
knobs in the qemu driver, doing it now proves that the API is
usable and also makes it easier for testing the qemu changes as
they are made.
* tools/virsh-domain.c (cmdBlockCommit): Add --active, --pivot,
and --finish options, modeled after blockcopy.
(blockJobImpl): Support --active flag.
* tools/virsh.pod (blockcommit): Document new flags.
(blockjob): Mention 2-phase commit interaction.
Signed-off-by: Eric Blake ebl...@redhat.com
---
tools/virsh-domain.c | 58 +---
tools/virsh.pod | 36 ++--
2 files changed, 76 insertions(+), 18 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 294b594..3db0bae 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1498,6 +1498,10 @@ blockJobImpl(vshControl *ctl, const vshCmd *cmd,
flags |= VIR_DOMAIN_BLOCK_COMMIT_DELETE;
if (vshCommandOptBool(cmd, keep-relative))
flags |= VIR_DOMAIN_BLOCK_COMMIT_RELATIVE;
+if (vshCommandOptBool(cmd, active) ||
+vshCommandOptBool(cmd, pivot) ||
+vshCommandOptBool(cmd, keep-overlay))
+flags |= VIR_DOMAIN_BLOCK_COMMIT_ACTIVE;
ret = virDomainBlockCommit(dom, path, base, top, bandwidth, flags);
break;
case VSH_CMD_BLOCK_JOB_COPY:
@@ -1598,13 +1602,18 @@ static const vshCmdOptDef opts_block_commit[] = {
.type = VSH_OT_DATA,
.help = N_(path of top file to commit from (default top of chain))
},
+{.name = active,
+ .type = VSH_OT_BOOL,
+ .help = N_(trigger two-stage active commit of top file)
+},
{.name = delete,
.type = VSH_OT_BOOL,
.help = N_(delete files that were successfully committed)
},
{.name = wait,
.type = VSH_OT_BOOL,
- .help = N_(wait for job to complete)
+ .help = N_(wait for job to complete
+(with --active, wait for job to sync))
},
{.name = verbose,
.type = VSH_OT_BOOL,
@@ -1612,7 +1621,15 @@ static const vshCmdOptDef opts_block_commit[] = {
},
{.name = timeout,
.type = VSH_OT_INT,
- .help = N_(with --wait, abort if copy exceeds timeout (in seconds))
+ .help = N_(implies --wait, abort if copy exceeds timeout (in seconds))
+},
+{.name = pivot,
+ .type = VSH_OT_BOOL,
+ .help = N_(implies --active --wait, pivot when commit is synced)
+},
+{.name = keep-overlay,
+ .type = VSH_OT_BOOL,
+ .help = N_(implies --active --wait, quit when commit is synced)
},
{.name = async,
.type = VSH_OT_BOOL,
@@ -1621,7 +1638,7 @@ static const vshCmdOptDef opts_block_commit[] = {
{.name = keep-relative,
.type = VSH_OT_BOOL,
.help = N_(keep the backing chain relative if it was relatively
-referenced if it was before)
+referenced before)
},
{.name = NULL}
};
@@ -1631,8 +1648,11 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom = NULL;
bool ret = false;
-bool blocking = vshCommandOptBool(cmd, wait);
bool verbose = vshCommandOptBool(cmd, verbose);
+bool pivot = vshCommandOptBool(cmd, pivot);
+bool finish = vshCommandOptBool(cmd, keep-overlay);
+bool active = vshCommandOptBool(cmd, active) || pivot || finish;
+bool blocking = vshCommandOptBool(cmd, wait);
int timeout = 0;
struct sigaction sig_action;
struct sigaction old_sig_action;
@@ -1643,7 +1663,12 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
bool quit = false;
int abort_flags = 0;
+blocking |= vshCommandOptBool(cmd, timeout) || pivot || finish;
if (blocking) {
+if (pivot finish) {
+vshError(ctl, %s, _(cannot mix --pivot and --keep-overlay));
+return false;
+}
if (vshCommandOptTimeoutToMs(ctl, cmd, timeout) 0)
return false;
if (vshCommandOptStringReq(ctl, cmd, path, path) 0)
@@ -1661,8 +1686,7 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
sigaction(SIGINT, sig_action, old_sig_action);
GETTIMEOFDAY(start);
-} else if (verbose || vshCommandOptBool(cmd, timeout) ||
- vshCommandOptBool(cmd, async)) {
+} else if (verbose || vshCommandOptBool(cmd, async)) {
vshError(ctl, %s, _(missing --wait option));
return false;
}
@@ -1694,6 +1718,8 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
if (verbose)
vshPrintJobProgress(_(Block Commit),
info.end - info.cur, info.end);
+if (active info.cur == info.end)
+break;
GETTIMEOFDAY(curr);
if (intCaught || (timeout
@@ -1720,7 +1746,25 @@ cmdBlockCommit(vshControl *ctl, const vshCmd *cmd)
[libvirt] [PATCH v3 0/5] Active commit
I still don't have qemu capability detection working reliably, but want to post this series right now so that it can be built into a scratch build containing Peter's and my changes. (Or put another way, I was testing what conflict resolutions are required - patch 2/5 (virsh) and 5/5 (qemu_driver) has some conflicts with Peter's addition of relative backing name; and I think the resolutions were fairly simple). These patches are tested on top of: https://www.redhat.com/archives/libvir-list/2014-June/msg00492.html I may still end up posting a v4 and/or pushing my series before Peter's, once I get capability detection working the way I want. Eric Blake (5): virsh: improve blockcopy UI virsh: expose new active commit controls blockcommit: update error messages related to block jobs blockcommit: track job type in xml blockcommit: turn on active commit docs/formatdomain.html.in | 20 +++--- docs/schemas/domaincommon.rng | 6 ++ src/conf/domain_conf.c | 26 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 56 --- src/qemu/qemu_hotplug.c| 2 +- src/qemu/qemu_process.c| 18 +++-- .../qemuxml2argv-disk-active-commit.xml| 37 ++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +- tests/qemuxml2xmltest.c| 1 + tools/virsh-domain.c | 83 +- tools/virsh.pod| 53 +- 13 files changed, 242 insertions(+), 69 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 4/5] blockcommit: track job type in xml
A future patch is going to wire up qemu active block commit jobs; but as they have similar events and are canceled/pivoted in the same way as block copy jobs, it is easiest to track all bookkeeping for the commit job by reusing the mirror element. This patch adds domain XML to track which job was responsible for creating a mirroring situation, and adds a job='copy' attribute to all existing uses of mirror. Along the way, it also massages the qemu monitor backend to read the new field in order to generate the correct type of libvirt job (even though it requires a future patch to actually cause a qemu event that can be reported as an active commit). * docs/schemas/domaincommon.rng: Enhance schema. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.h (_virDomainDiskDef): Add a field. * src/conf/domain_conf.c (virDomainBlockJobType): String conversion. (virDomainDiskDefParseXML): Parse job type. (virDomainDiskDefFormat): Output job type. * src/qemu/qemu_process.c (qemuProcessHandleBlockJob): Distinguish active from regular commit. * src/qemu/qemu_driver.c (qemuDomainBlockCopy): Set job type. (qemuDomainBlockPivot, qemuDomainBlockJobImpl): Clean up job type on completion. * tests/qemuxml2xmloutdata/qemuxml2xmlout-disk-mirror-old.xml: Update tests. * tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Likewise. * tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml: New file. * tests/qemuxml2xmltest.c (mymain): Drive new test. Signed-off-by: Eric Blake ebl...@redhat.com --- docs/formatdomain.html.in | 20 ++-- docs/schemas/domaincommon.rng | 6 src/conf/domain_conf.c | 24 ++ src/conf/domain_conf.h | 1 + src/qemu/qemu_driver.c | 3 ++ src/qemu/qemu_process.c| 18 +++ .../qemuxml2argv-disk-active-commit.xml| 37 ++ .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 4 +-- .../qemuxml2xmlout-disk-mirror-old.xml | 4 +-- tests/qemuxml2xmltest.c| 1 + 10 files changed, 98 insertions(+), 20 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-disk-active-commit.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 1b6ced8..512132d 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1878,16 +1878,18 @@ /dd dtcodemirror/code/dt dd -This element is present if the hypervisor has started a block -copy operation (via the codevirDomainBlockRebase/code -API), where the mirror location in the codesource/code -sub-element will eventually have the same contents as the -source, and with the file format in the +This element is present if the hypervisor has started a +long-running block job operation, where the mirror location in +the codesource/code sub-element will eventually have the +same contents as the source, and with the file format in the sub-element codeformat/code (which might differ from the -format of the source). The details of the codesource/code -sub-element are determined by the codetype/code attribute -of the mirror, similar to what is done for the -overall codedisk/code device element. If +format of the source). The codejob/code attribute +mentions which API started the operation (copy for +the codevirDomainBlockRebase/code API, or commit for +the codevirDomainBlockCommit/code API). The details of +the codesource/code sub-element are determined by +the codetype/code attribute of the mirror, similar to what +is done for the overall codedisk/code device element. If attribute codeready/code is present, then it is known the disk is ready to pivot; otherwise, the disk is probably still copying. For now, this element only valid in output; it is diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 6cc922c..3badb6d 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4187,6 +4187,12 @@ /optional /group group + attribute name='job' +choice + valuecopy/value + valueactive-commit/value +/choice + /attribute interleave ref name=diskSource/ optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index be81dbe..fbe9fa4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -760,6 +760,12 @@ VIR_ENUM_IMPL(virDomainDiskDiscard, VIR_DOMAIN_DISK_DISCARD_LAST, unmap, ignore) +/* Internal mapping: subset of block job types that can be present in + * mirror XML (remaining types are not two-phase). */
[libvirt] [PATCH v3 3/5] blockcommit: update error messages related to block jobs
A future patch will add two-phase block commit jobs; as the
mechanism for managing them is similar to managing a block copy
job, existing errors should be made generic enough to occur
for either job type.
* src/conf/domain_conf.c (virDomainHasDiskMirror): Update
comment.
* src/qemu/qemu_driver.c (qemuDomainDefineXML)
(qemuDomainSnapshotCreateXML, qemuDomainRevertToSnapshot)
(qemuDomainBlockJobImpl, qemuDomainBlockCopy): Update error
message.
* src/qemu/qemu_hotplug.c (qemuDomainDetachDiskDevice): Likewise.
Signed-off-by: Eric Blake ebl...@redhat.com
---
src/conf/domain_conf.c | 2 +-
src/qemu/qemu_driver.c | 10 +-
src/qemu/qemu_hotplug.c | 2 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index ff2d447..be81dbe 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10379,7 +10379,7 @@ virDomainDiskRemoveByName(virDomainDefPtr def, const
char *name)
}
/* Return true if VM has at least one disk involved in a current block
- * copy job (that is, with a mirror element in the disk xml). */
+ * copy/commit job (that is, with a mirror element in the disk xml). */
bool
virDomainHasDiskMirror(virDomainObjPtr vm)
{
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 3f42af9..3399b2a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -6260,7 +6260,7 @@ static virDomainPtr qemuDomainDefineXML(virConnectPtr
conn, const char *xml)
def = NULL;
if (virDomainHasDiskMirror(vm)) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s,
- _(domain has active block copy job));
+ _(domain has active block job));
virDomainObjAssignDef(vm, NULL, false, NULL);
goto cleanup;
}
@@ -13464,7 +13464,7 @@ qemuDomainSnapshotCreateXML(virDomainPtr domain,
}
if (virDomainHasDiskMirror(vm)) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s,
- _(domain has active block copy job));
+ _(domain has active block job));
goto cleanup;
}
@@ -14074,7 +14074,7 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if (virDomainHasDiskMirror(vm)) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE, %s,
- _(domain has active block copy job));
+ _(domain has active block job));
goto cleanup;
}
@@ -15083,7 +15083,7 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
if (mode == BLOCK_JOB_PULL disk-mirror) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
- _(disk '%s' already in active block copy job),
+ _(disk '%s' already in active block job),
disk-dst);
goto endjob;
}
@@ -15318,7 +15318,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
disk = vm-def-disks[idx];
if (disk-mirror) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
- _(disk '%s' already in active block copy job),
+ _(disk '%s' already in active block job),
disk-dst);
goto endjob;
}
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index cd43dd5..7289055 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2986,7 +2986,7 @@ qemuDomainDetachDiskDevice(virQEMUDriverPtr driver,
if (detach-mirror) {
virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
- _(disk '%s' is in an active block copy job),
+ _(disk '%s' is in an active block job),
detach-dst);
goto cleanup;
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 5/5] blockcommit: turn on active commit
With this in place, I can (finally!) now do:
virsh blockcommit $dom vda --shallow --wait --verbose --pivot
and watch qemu shorten the backing chain by one, followed by
libvirt automatically updating the dumpxml output, effectively
undoing the work of virsh snapshot-commit --no-metadata --disk-only.
Commit is S much faster than blockpull, when I'm still fairly
close in time to when the temporary qcow2 wrapper file was created
via a snapshot operation!
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Implement live
commit to regular files.
Signed-off-by: Eric Blake ebl...@redhat.com
---
src/qemu/qemu_driver.c | 43 ++-
1 file changed, 38 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 728aa85..60dc9c6 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15529,9 +15529,11 @@ qemuDomainBlockCommit(virDomainPtr dom,
char *topPath = NULL;
char *basePath = NULL;
char *backingPath = NULL;
+virStorageSourcePtr mirror = NULL;
-/* XXX Add support for COMMIT_ACTIVE, COMMIT_DELETE */
+/* XXX Add support for COMMIT_DELETE */
virCheckFlags(VIR_DOMAIN_BLOCK_COMMIT_SHALLOW |
+ VIR_DOMAIN_BLOCK_COMMIT_ACTIVE |
VIR_DOMAIN_BLOCK_COMMIT_RELATIVE, -1);
if (!(vm = qemuDomObjFromDomain(dom)))
@@ -15578,13 +15580,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
top_parent)))
goto endjob;
-/* FIXME: qemu 2.0 supports active commit, but as a two-stage
- * process; qemu 2.1 is further improving active commit. We need
- * to start supporting it in libvirt. */
if (topSource == disk-src) {
/* We assume that no one will backport qemu 2.0 active commit
* to an earlier qemu without also backporting the block job
- * ready event; but this makes sure of that fact */
+ * ready event; but this makes sure of that fact.
+ *
+ * XXX Also need to check other capability bit(s): qemu 1.7
+ * supports async blockjob but not active commit; and qemu 2.0
+ * active commit misbehaves on 0-byte file. */
if (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_BLOCKJOB_ASYNC)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(active commit not supported with this QEMU
binary));
@@ -15597,6 +15600,14 @@ qemuDomainBlockCommit(virDomainPtr dom,
disk-dst);
goto endjob;
}
+if (disk-mirror) {
+virReportError(VIR_ERR_BLOCK_COPY_ACTIVE,
+ _(disk '%s' already in active block job),
+ disk-dst);
+goto endjob;
+}
+if (VIR_ALLOC(mirror) 0)
+goto endjob;
} else if (flags VIR_DOMAIN_BLOCK_COMMIT_ACTIVE) {
virReportError(VIR_ERR_INVALID_ARG,
_(active commit requested but '%s' is not active),
@@ -15667,6 +15678,21 @@ qemuDomainBlockCommit(virDomainPtr dom,
}
}
+/* For an active commit, clone enough of the base to act as the mirror */
+if (mirror) {
+/* XXX Support network commits */
+if (baseSource-type != VIR_STORAGE_TYPE_FILE
+baseSource-type != VIR_STORAGE_TYPE_BLOCK) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(active commit to non-file not supported yet));
+goto endjob;
+}
+mirror-type = baseSource-type;
+if (VIR_STRDUP(mirror-path, baseSource-path) 0)
+goto endjob;
+mirror-format = baseSource-format;
+}
+
/* Start the commit operation. Pass the user's original spelling,
* if any, through to qemu, since qemu may behave differently
* depending on whether the input was specified as relative or
@@ -15678,6 +15704,12 @@ qemuDomainBlockCommit(virDomainPtr dom,
bandwidth);
qemuDomainObjExitMonitor(driver, vm);
+if (ret == 0 mirror) {
+disk-mirror = mirror;
+mirror = NULL;
+disk-mirrorJob = VIR_DOMAIN_BLOCK_JOB_TYPE_ACTIVE_COMMIT;
+}
+
endjob:
if (ret 0 clean_access) {
/* Revert access to read-only, if possible. */
@@ -15688,6 +15720,7 @@ qemuDomainBlockCommit(virDomainPtr dom,
top_parent,
VIR_DISK_CHAIN_READ_ONLY);
}
+virStorageSourceFree(mirror);
if (!qemuDomainObjEndJob(driver, vm))
vm = NULL;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] Active commit
On 06/11/2014 10:27 AM, Eric Blake wrote: I still don't have qemu capability detection working reliably, but want to post this series right now so that it can be built into a scratch build containing Peter's and my changes. (Or put another way, I was testing what conflict resolutions are required - patch 2/5 (virsh) and 5/5 (qemu_driver) has some conflicts with Peter's addition of relative backing name; and I think the resolutions were fairly simple). These patches are tested on top of: https://www.redhat.com/archives/libvir-list/2014-June/msg00492.html I may still end up posting a v4 and/or pushing my series before Peter's, once I get capability detection working the way I want. Phooey, I forgot to send my v3 changelog. Eric Blake (5): virsh: improve blockcopy UI new in v3 virsh: expose new active commit controls resolves Peter's comments in v2 about ?:, rename --finish to --keep-overlay, and copy UI enhancements from 1/5. Conflict resolution with --keep-relative. blockcommit: update error messages related to block jobs unchanged from v2 blockcommit: track job type in xml Reflow html source, tweak comment about why only 2 of the 5 job types are mapped to string blockcommit: turn on active commit Conflict resolution with --keep-relative; still needs proper qemu capability detection -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/4] virnetdev: Introduce virNetDevGetLinkInfo
On 06/11/2014 03:19 AM, Michal Privoznik wrote:
+{
+int ret = -1;
+char *path = NULL;
+char *buf = NULL;
+char *tmp;
+int tmp_state;
s/int/virInterfaceState/
In fact this is intentional. Remember Eric's TypeFromString() patches?
The problem is, one can't be sure if compiler decides on using unsigned
or signed integer to represent an enum. If it decides to use an unsigned
int (IIRC that's the case of older gcc) then comparison a few lines
below will never be true.
But at the least, you can do:
int tmp_state /* virInterfaceState */
to make it easier for later readers to know valid values to store in the
int.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH V8 2/2] libxl: add migration support
On 06/11/2014 11:13 AM, Jim Fehlig wrote: John Ferlan wrote: This patch has resulted in a new Coverity warnings (looking at them was just lower on my list of things to do lately)... Anyway - see libxlDoMigrateReceive() and libxlDomainMigrationFinish() for the details... Should have looked at this before responding to your patch to fix the warnings. Does that patch fix all the issues noted below? Regards, Jim Yes it does - I figured they were low hanging fruit so I just sent the patch... Responding to it gives recent context rather than searching out the git commit id or the list entry in the archives and pointing at it. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] build: remove duplicit warning suppression
On 06/11/2014 03:00 AM, Ján Tomko wrote: These warnings have alread been added to $dontwarn. s/alread/already/ --- m4/virt-compile-warnings.m4 | 3 --- 1 file changed, 3 deletions(-) ACK diff --git a/m4/virt-compile-warnings.m4 b/m4/virt-compile-warnings.m4 index 574fbc4..fb82238 100644 --- a/m4/virt-compile-warnings.m4 +++ b/m4/virt-compile-warnings.m4 @@ -119,9 +119,6 @@ AC_DEFUN([LIBVIRT_COMPILE_WARNINGS],[ # ideally we'd turn many of them on dontwarn=$dontwarn -Wfloat-equal dontwarn=$dontwarn -Wdeclaration-after-statement -dontwarn=$dontwarn -Wcast-qual -dontwarn=$dontwarn -Wconversion -dontwarn=$dontwarn -Wsign-conversion dontwarn=$dontwarn -Wpacked dontwarn=$dontwarn -Wunused-macros dontwarn=$dontwarn -Woverlength-strings -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] build: remove ssp-buffer-size
On 06/11/2014 03:00 AM, Ján Tomko wrote: This option only makes sense for -fstack-protector. With -fstack-protector-all or -fstack-protector-strong, functions are protected regardless of buffer size. https://bugzilla.redhat.com/show_bug.cgi?id=1105456 --- m4/virt-compile-warnings.m4 | 13 - 1 file changed, 4 insertions(+), 9 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv4 01/21] security: Don't skip labelling for network disks
On 06/11/2014 05:45 AM, Peter Krempa wrote: A network disk might actually be backed by local storage. Also the path iterator actually handles networked disks well now so remove the code that skips the labelling in dac and selinux security driver. --- src/security/security_dac.c | 3 --- src/security/security_selinux.c | 3 --- 2 files changed, 6 deletions(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] maint: exempt graphic binaries from syntax check
Eric Blake wrote: On 06/11/2014 02:56 AM, Martin Kletzander wrote: On Tue, Jun 10, 2014 at 01:39:03PM -0600, Eric Blake wrote: Roman Bogorodskiy reported a syntax-check failure when using FreeBSD; complaining that: prohibit_empty_first_line tools/libvirt_win_icon_16x16.ico:1: tools/libvirt_win_icon_32x32.ico:1: tools/libvirt_win_icon_48x48.ico:1: tools/libvirt_win_icon_64x64.ico:1: maint.mk: Prohibited empty first line In reality, the first 'line' of that file is NOT empty; but since it is a binary file, awk is not required to handle it gracefully. The simplest solution is to exempt all image files from syntax checks in the first place - after all, we only store them in git because they are inconvenient to regenerate, but they are not our preferred format for making modifications, and syntax check should only cover files that we are likely to modify. * cfg.mk (VC_LIST_ALWAYS_EXCLUDE_REGEX): Exempt images. (exclude_file_name_regexp--sc_prohibit_empty_lines_at_EOF): Simplify. (exclude_file_name_regexp--sc_trailing_blank): Likewise. ACK Pushed. Thanks, it fixes the problem for me. Roman Bogorodskiy pgpT5etUSys6U.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv3 07/36] storage: Switch metadata crawler to use storage driver to get unique path
Peter Krempa wrote:
On 06/09/14 16:07, Roman Bogorodskiy wrote:
Peter Krempa wrote:
On 06/07/14 20:35, Roman Bogorodskiy wrote:
Peter Krempa wrote:
Use the virStorageFileGetUniqueIdentifier() function to get a unique
identifier regardless of the target storage type instead of relying on
canonicalize_path().
A new function that checks whether we support a given image is
introduced to avoid errors for unimplemented backends.
---
Notes:
Version 3:
- fixed typo
- ACKed by Eric
src/storage/storage_driver.c | 77
+++-
1 file changed, 54 insertions(+), 23 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index f92a553..5c4188f 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3112,49 +3135,63 @@
virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
int fd;
int ret = -1;
struct stat st;
+const char *uniqueName;
virStorageSourcePtr backingStore = NULL;
int backingFormat;
-VIR_DEBUG(path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d
probe=%d,
- src-path, canonPath, NULLSTR(src-relDir), src-format,
+VIR_DEBUG(path=%s dir=%s format=%d uid=%d gid=%d probe=%d,
+ src-path, NULLSTR(src-relDir), src-format,
(int)uid, (int)gid, allow_probe);
-if (virHashLookup(cycle, canonPath)) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(backing store for %s is self-referential),
- src-path);
+/* exit if we can't load information about the current image */
+if (!virStorageFileSupportsBackingChainTraversal(src))
+return 0;
After this change I get virstrogetest failing on FreeBSD, which doesn't
support any of the file storage backends currently:
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 806c06400 (LWP 100061/virstoragetest)]
0x00410088 in mymain () at virstoragetest.c:937
937 TEST_LOOKUP(3, qcow2, chain-backingStore-path,
chain-backingStore,
Current language: auto; currently minimal
(gdb) p chain
$1 = 0x806c7b100
(gdb) p chain-backingStore
$2 = 0x0
(gdb)
Hmm, so FreeBSD doesn't currently compile the storage driver? We should
probably look into enabling it. All the stuff that was done by the code
was compiling just fine on FreeBSD previously so enabling just the local
filesystem storage backend should work well. I'll have a look what that
would include.
virstorage.c test calls testStorageFileGetMetadata() which calls
virStorageFileGetMetadata().
Inside of that, the call sequence is:
virStorageFileGetMetadata - virStorageFileGetMetadataRecurse
virStorageFileGetMetadataRecurse then checks
!virStorageFileSupportsBackingChainTraversal() and returns 0.
virStorageFileSupportsBackingChainTraversal tries to initialise backend
using virStorageFileBackendForTypeInternal().
virStorageFileBackendForTypeInternal loops through fileBackends which
looks this way:
static virStorageFileBackendPtr fileBackends[] = {
#if WITH_STORAGE_FS
virStorageFileBackendFile,
virStorageFileBackendBlock,
#endif
#if WITH_STORAGE_GLUSTER
virStorageFileBackendGluster,
#endif
NULL
};
FreeBSD doesn't support neither WITH_STORAGE_GLUSTER nor WITH_STORAGE_FS. So
we end up having chain-backingStore == NULL.
PS I may be wrong here as it's my first experience with the storage code
and I haven't yet spend much time on it.
Yes, the problem is that VIR_STORAGE_FS isn't enabled on freebsd.
I don't know whether it's worth analyzing the code to find out if it can
be enabled on freebsd or not. (the needed portions certainly can be
enabled, but some of the other portions might be problematic.
Providing a fully functional virStorageBackendFileSystem backend will
certainly require some effort, at least getmntent_r() part and mouting
part needs to be re-implemnented for BSD.
I'll probably take a look later.
At any rate, I'm working on a refactor of the test that will allow to
skip the test (and remove a few ugly pieces of code)
Thanks!
Roman Bogorodskiy
pgpGhdCYunwEZ.pgp
Description: PGP signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: include bhyve in virsh -V output
Eric Blake wrote: On 06/10/2014 12:26 PM, Roman Bogorodskiy wrote: Add 'Bhyve' in hypervisor list reported by 'virsh -V' if it's compiled it. --- tools/virsh.c | 3 +++ 1 file changed, 3 insertions(+) ACK Pushed, thanks! Roman Bogorodskiy pgpVB8oonTwzn.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] libxl: Resolve Coverity warnings
On 06/11/2014 11:08 AM, Jim Fehlig wrote: John Ferlan wrote: Resolve two Coverity issues introduced by commit id '9b8d6e1e' Signed-off-by: John Ferlan jfer...@redhat.com --- src/libxl/libxl_migration.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) ACK. Regards, Jim Tks, I have pushed 3/3 John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] Active commit
On 11/06/14 10:27 -0600, Eric Blake wrote: I still don't have qemu capability detection working reliably, but want to post this series right now so that it can be built into a scratch build containing Peter's and my changes. (Or put another way, I was testing what conflict resolutions are required - patch 2/5 (virsh) and 5/5 (qemu_driver) has some conflicts with Peter's addition of relative backing name; and I think the resolutions were fairly simple). These patches are tested on top of: https://www.redhat.com/archives/libvir-list/2014-June/msg00492.html I may still end up posting a v4 and/or pushing my series before Peter's, once I get capability detection working the way I want. Eric Blake (5): virsh: improve blockcopy UI virsh: expose new active commit controls blockcommit: update error messages related to block jobs blockcommit: track job type in xml blockcommit: turn on active commit I've tested this with the following script and it seems that specifying base and top explicitly does not work. Using --shallow does work for me though. Running the included script yields the following output: $ sudo bash /home/alitke/test/test.sh Formatting 'base.img', fmt=raw size=1073741824 Formatting 'snap1.img', fmt=qcow2 size=1073741824 backing_file='base.img' backing_fmt='raw' encryption=off cluster_size=65536 lazy_refcounts=off Formatting 'snap2.img', fmt=qcow2 size=1073741824 backing_file='snap1.img' backing_fmt='qcow2' encryption=off cluster_size=65536 lazy_refcounts=off image: /tmp/snap2.img file format: qcow2 virtual size: 1.0G (1073741824 bytes) disk size: 196K cluster_size: 65536 backing file: snap1.img (actual path: /tmp/snap1.img) backing file format: qcow2 Format specific information: compat: 1.1 lazy refcounts: false Domain testvm1 created from /dev/stdin error: invalid argument: could not find image '/tmp/snap2.img' in chain for '/tmp/snap2.img' Domain testvm1 destroyed -- #!/bin/sh rm -f base.img snap1.img snap2.img # base.img - snap1.img - snap2.img qemu-img create -f raw base.img 1G qemu-img create -f qcow2 -b base.img -o backing_fmt=raw snap1.img qemu-img create -f qcow2 -b snap1.img -o backing_fmt=qcow2 snap2.img chcon -t svirt_image_t base.img snap1.img snap2.img chmod 666 base.img snap1.img snap2.img qemu-img info $PWD/snap2.img virsh create /dev/stdin EOF domain type='kvm' nametestvm1/name memory unit='MiB'256/memory vcpu1/vcpu os type arch='x86_64'hvm/type /os devices disk type='file' device='disk' driver name='qemu' type='qcow2'/ source file='$PWD/snap2.img'/ target dev='vda' bus='virtio'/ /disk graphics type='vnc'/ /devices /domain EOF base=$PWD/snap1.img top=$PWD/snap2.img virsh blockcommit testvm1 vda 0 $base $top --wait --verbose --pivot virsh destroy testvm1 -- Adam Litke -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/5] Active commit
On 06/11/2014 01:53 PM, Adam Litke wrote: I've tested this with the following script and it seems that specifying base and top explicitly does not work. Using --shallow does work for me though. Running the included script yields the following output: $ sudo bash /home/alitke/test/test.sh Formatting 'base.img', fmt=raw error: invalid argument: could not find image '/tmp/snap2.img' in chain for '/tmp/snap2.img' base=$PWD/snap1.img top=$PWD/snap2.img virsh blockcommit testvm1 vda 0 $base $top --wait --verbose --pivot Okay, I see the flaw - when looking up an explicit top, we aren't starting from the active layer. Try omitting the --top argument as a workaround. Meanwhile, this patch should fix it: diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c index 60dc9c6..aded8e0 100644 --- i/src/qemu/qemu_driver.c +++ w/src/qemu/qemu_driver.c @@ -15575,7 +15575,7 @@ qemuDomainBlockCommit(virDomainPtr dom, topSource = disk-src; else if (virStorageFileParseChainIndex(disk-dst, top, topIndex) 0 || !(topSource = virStorageFileChainLookup(disk-src, - disk-src-backingStore, + disk-src, top, topIndex, top_parent))) goto endjob; -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
