[libvirt] Host OS, Storage Info
Hi, Is there any way to get host OS information and host Storage in formations using libvirt API...? Rgds -Sijo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH 0/5] another disk refactoring
On Wed, May 21, 2014 at 22:50:43 -0600, Eric Blake wrote: I'm about to reuse the mirror sub-element of disk for my pending series on active block commit, but to do that, I wanted to get some refactoring out of the way first. Patches 1-4 are pretty straightforward, and 5 may be controversial (I'm sending it now, even though it still needs more work, to make sure I'm not doing something bad). I'm aware that this will probably cause some rebase pain for Peter's work with gluster, but at the same time, it will ease some of the work that Benoit wants for introducing quorums. I think we all agree that we need to do this refactoring. However, the timing is not very good given that Peter already has lots of patches mostly ready to be submitted to the list while Benoit's work on quorums is only in its XML design phase. In other words, we should first wait for Peter's patches and Benoit may in the mean time work on his patches which will count with your refactoring. But pushing this refactoring now is not going to help anyone. In other words, NACK to patches 1-3 until patches for gluster snapshots and relative backing chains land in. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] conf: fix backing store parse off-by-one
On Wed, May 21, 2014 at 22:06:08 -0600, Eric Blake wrote: Commit 546154e parses the type attribute from a backingStore element, but forgot that the earlier commit 9673418 added a placeholder element in the same 1.2.3 release; as a result, the C code was mistakenly allowing none as a type. Similarly, the same commit allows none as the format sub-element type, even though that has been a placeholder since the 0.10.2 release with commit f772b3d. * src/conf/domain_conf.c (virDomainDiskBackingStoreParse): Require non-zero types. Signed-off-by: Eric Blake ebl...@redhat.com --- Maybe worth addressing in a later patch: the RelaxNG grammar currently requires a format and backingStore subelement to any non-empty backingStore, and the C code matches this requirement. However, we should probably make both of them optional, to represent the case where the user is requesting that we perform a probe to complete the backing chain details. Right, however, this change will only make sense when we actually add support for user-supplied backing chains so implementing this part earlier is useless and perhaps even confusing. ACK Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemuSetupCgroupForVcpu: s/virProcessInfoSetAffinity/virProcessSetAffinity/
In the f56c773bf we've made the substitution but forgot to fix one comment which is still referring to the old name. This may be potentially misleading. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_cgroup.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 911936e..b1bfb5a 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -904,8 +904,7 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) } /* We are trying to setup cgroups for CPU pinning, which can also be done - * with virProcessInfoSetAffinity, thus the lack of cgroups is not fatal - * here. + * with virProcessSetAffinity, thus the lack of cgroups is not fatal here. */ if (priv-cgroup == NULL) return 0; -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemuSetupCgroupForVcpu: s/virProcessInfoSetAffinity/virProcessSetAffinity/
On Thu, May 22, 2014 at 12:32:02PM +0200, Michal Privoznik wrote: In the f56c773bf we've made the substitution but forgot to fix one comment which is still referring to the old name. This may be potentially misleading. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_cgroup.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 911936e..b1bfb5a 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -904,8 +904,7 @@ qemuSetupCgroupForVcpu(virDomainObjPtr vm) } /* We are trying to setup cgroups for CPU pinning, which can also be done - * with virProcessInfoSetAffinity, thus the lack of cgroups is not fatal - * here. + * with virProcessSetAffinity, thus the lack of cgroups is not fatal here. */ if (priv-cgroup == NULL) return 0; ACK, trivial 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 2/3] qemu: Fix specifying char devs for PPC
QEMU ppce500 board uses the old style -serial options. Other PPC boards don't give any way to explicitly wire in a -chardev except pseries which uses -device spapr-vty with -chardev. --- src/qemu/qemu_capabilities.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 03d8842..1cc37ad 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3466,13 +3466,17 @@ virQEMUCapsSupportsChardev(virDomainDefPtr def, !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) return false; -if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64)) +if ((def-os.arch != VIR_ARCH_ARMV7L) (def-os.arch != VIR_ARCH_AARCH64) + (def-os.arch != VIR_ARCH_PPC) (def-os.arch != VIR_ARCH_PPC64)) return true; /* This may not be true for all ARM machine types, but at least * the only supported non-virtio serial devices of vexpress and versatile - * don't have the -chardev property wired up. */ + * don't have the -chardev property wired up. + * For PPC machines, only pseries need -device spapr-vty with -chardev */ return (chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_VIRTIO_MMIO || (chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE - chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO)); + chr-targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) || +(chr-deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL + chr-info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_SPAPRVIO)); } -- 1.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] change machine name ppce500v2 as ppce500
---
docs/schemas/domaincommon.rng| 2 +-
tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args | 2 +-
tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml | 2 +-
tests/testutilsqemu.c| 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4249ed5..af67123 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -371,7 +371,7 @@
valueg3beige/value
valuemac99/value
valueprep/value
-valueppce500v2/value
+valueppce500/value
/choice
/attribute
/optional
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
b/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
index fd7e994..5d6dc45 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
+++ b/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args
@@ -1,5 +1,5 @@
LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
-/usr/bin/qemu-system-ppc -S -M ppce500v2 -m 256 -smp 1 -nographic \
+/usr/bin/qemu-system-ppc -S -M ppce500 -m 256 -smp 1 -nographic \
-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \
-kernel /media/ram/uImage -initrd /media/ram/ramdisk \
-append 'root=/dev/ram rw console=ttyS0,115200' -dtb /media/ram/test.dtb \
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml
b/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml
index 3674621..04f0eb6 100644
--- a/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml
+++ b/tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml
@@ -5,7 +5,7 @@
currentMemory unit='KiB'262144/currentMemory
vcpu placement='static'1/vcpu
os
-type arch='ppc' machine='ppce500v2'hvm/type
+type arch='ppc' machine='ppce500'hvm/type
kernel/media/ram/uImage/kernel
initrd/media/ram/ramdisk/initrd
cmdlineroot=/dev/ram rw console=ttyS0,115200/cmdline
diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c
index a8884ba..7e24909 100644
--- a/tests/testutilsqemu.c
+++ b/tests/testutilsqemu.c
@@ -91,7 +91,7 @@ static int testQemuAddPPCGuest(virCapsPtr caps)
static const char *machine[] = { g3beige,
mac99,
prep,
- ppce500v2 };
+ ppce500 };
virCapsGuestMachinePtr *machines = NULL;
virCapsGuestPtr guest;
--
1.8.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] tests: add test case for -serial option for ppce500
--- .../qemuxml2argv-ppce500-serial.args | 7 ++ .../qemuxml2argv-ppce500-serial.xml| 26 ++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 34 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ppce500-serial.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ppce500-serial.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppce500-serial.args b/tests/qemuxml2argvdata/qemuxml2argv-ppce500-serial.args new file mode 100644 index 000..c7b4819 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ppce500-serial.args @@ -0,0 +1,7 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc -S -M ppce500 -m 256 -smp 1 -nographic \ +-chardev socket,id=charmonitor,path=/tmp/test-monitor,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline -no-acpi -boot c \ +-kernel /media/ram/uImage -initrd /media/ram/ramdisk \ +-append 'root=/dev/ram rw console=ttyS0,115200' \ +-usb -net none -serial pty -parallel none diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ppce500-serial.xml b/tests/qemuxml2argvdata/qemuxml2argv-ppce500-serial.xml new file mode 100644 index 000..397aadc --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ppce500-serial.xml @@ -0,0 +1,26 @@ +domain type='kvm' + nameQEMUGuest1/name + memory unit='KiB'262144/memory + currentMemory unit='KiB'262144/currentMemory + vcpu placement='static'1/vcpu + os +type arch='ppc' machine='ppce500'hvm/type +kernel/media/ram/uImage/kernel +initrd/media/ram/ramdisk/initrd +cmdlineroot=/dev/ram rw console=ttyS0,115200/cmdline + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu-system-ppc/emulator +serial type='pty' + target port='0'/ +/serial +console type='pty' + target type='serial' port='0'/ +/console +memballoon model='virtio'/ + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1ea7bf8..b2aa22a 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1279,6 +1279,7 @@ mymain(void) QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM); DO_TEST(ppc-dtb, QEMU_CAPS_KVM, QEMU_CAPS_DTB); +DO_TEST(ppce500-serial, QEMU_CAPS_KVM, QEMU_CAPS_DRIVE, QEMU_CAPS_CHARDEV); DO_TEST(tpm-passthrough, QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); -- 1.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] use -serial for ppce500 board and add test case
Machine name ppce500 is used to replace ppce500v2 supported by QEMU. QEMU ppce500 board uses the old style -serial options. Test case ppce500-serial is used to verify this change. Olivia Yin (3): change machine name ppce500v2 as ppce500 qemu: Fix specifying char devs for PPC tests: add test case for -serial option for ppce500 docs/schemas/domaincommon.rng | 2 +- src/qemu/qemu_capabilities.c | 10 ++--- tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.args | 2 +- tests/qemuxml2argvdata/qemuxml2argv-ppc-dtb.xml| 2 +- .../qemuxml2argv-ppce500-serial.args | 7 ++ .../qemuxml2argv-ppce500-serial.xml| 26 ++ tests/qemuxml2argvtest.c | 1 + tests/testutilsqemu.c | 2 +- 8 files changed, 45 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ppce500-serial.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ppce500-serial.xml -- 1.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/4] qemu: fix broken RTC_CHANGE event when clock offset='variable'
(the cover letter from V1 has been preserved here with small updates) This patch series is all about: https://bugzilla.redhat.com/show_bug.cgi?id=964177 which points out a flaw in qemu's RTC_CHANGE event when the base for the guest's real time clock was given as an explicit date (rather than as 'utc' or 'localtime'). Patch 3/4 explains what qemu does, and how this patch fixes it (for the case of basis='utc' - an additional fix to properly support basis='localtime', both for RTC_CHANGE and when migrating across timezone/DST boundaries, is in Patch 4/4). (NB: the flawed RTC_CHANGE offset has been in QEMU for so long now that it has been documented and cannot be changed, so libvirt must work around it) In the meantime, the fix for basis='localtime' required that we learn the offset of localtime from utc, and that seems like something we might want to do for other reasons, so Patch 1/4 adds a new utility function do get that value that is (I hope!) POSIX compliant. Since the original patch to fix this problem was incorrect, and the new patch doesn't use any of its code, Patch 2/4 reverts that patch completely. Note that I have tested this patch by starting a domain with several variations of clock parameters (in a locale that is currently at UTC+3) and using the following short script to add and remove seconds from the guest's RTC while watching both the clock line in /var/run/libvirt/qemu/$domain.xml and the offset value sent in libvirt's VIR_DOMAIN_EVENT_ID_RTC_CHANGE event (using examples/domain-events/events-c/event-test from a libvirt source tree that has been built). All cases appear to maintain the adjustment in the status properly, as well as sending the correct value to any event handler. Here is the script I used to add/remove time from the RTC: #!/bin/sh old=$(hwclock --show | cut -f1-7 -d' ') oldabs=$(date +%s --date=$old) newabs=$(expr $oldabs + $1) new=$(date --date=@$newabs) echo Old: $oldabs $old echo New: $newabs $new hwclock --set --date=@$newabs Laine Stump (4): util: new function virTimeLocalOffsetFromUTC Revert qemu: Report the offset from host UTC for RTC_CHANGE event qemu: fix RTC_CHANGE event for clock offset='variable' basis='utc'/ qemu: fix clock offset='variable' basis='localtime'/ src/conf/domain_conf.c | 32 +--- src/conf/domain_conf.h | 8 +--- src/libvirt_private.syms | 1 + src/qemu/qemu_command.c | 43 ++- src/qemu/qemu_process.c | 34 +++--- src/util/virtime.c | 41 - src/util/virtime.h | 5 +++-- tests/virtimetest.c | 44 8 files changed, 155 insertions(+), 53 deletions(-) -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/4] util: new function virTimeLocalOffsetFromUTC
Since there isn't a single libc API to get this value, this patch
supplies one which gets the value by grabbing current UTC, then
converting that into a struct tm with localtime_r(), then back to a
time_t using mktime; it again does the same operation, but using
gmtime_r() instead (for UTC). It then subtracts utc time from the
localtime, and finally adjusts if dst is set in the localtime timeinfo
(because for some reason mktime doesn't take that into account).
This function should be POSIX-compliant, and is threadsafe, but not
async signal safe. If it was ever necessary to know this value in a
child process, we could cache it with a one-time init function when
libvirtd starts, then just supply the cached value, but that
complexity isn't needed for current usage.
---
Change from V1: add test cases with TZ set to different values (if
someone knows how to force DST on/off, I would gladly add some test
cases for this as well).
src/libvirt_private.syms | 1 +
src/util/virtime.c | 41 -
src/util/virtime.h | 5 +++--
tests/virtimetest.c | 44
4 files changed, 88 insertions(+), 3 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c3332c9..cb635cd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1984,6 +1984,7 @@ virTimeFieldsNow;
virTimeFieldsNowRaw;
virTimeFieldsThen;
virTimeFieldsThenRaw;
+virTimeLocalOffsetFromUTC;
virTimeMillisNow;
virTimeMillisNowRaw;
virTimeStringNow;
diff --git a/src/util/virtime.c b/src/util/virtime.c
index caa4e24..c487eba 100644
--- a/src/util/virtime.c
+++ b/src/util/virtime.c
@@ -1,7 +1,7 @@
/*
* virtime.c: Time handling functions
*
- * Copyright (C) 2006-2012 Red Hat, Inc.
+ * Copyright (C) 2006-2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -344,3 +344,42 @@ char *virTimeStringThen(unsigned long long when)
return ret;
}
+
+/**
+ * virTimeLocalOffsetFromUTC:
+ *
+ * This function is threadsafe, but is *not* async signal safe (due to
+ * localtime_r()).
+ *
+ * @offset: pointer to time_t that will difference between localtime
+ * and UTC in seconds.
+ *
+ * Returns 0 on success, -1 on error with error reported
+ */
+int
+virTimeLocalOffsetFromUTC(time_t *offset)
+{
+struct tm gmtimeinfo, localtimeinfo;
+time_t current, utc, local;
+
+if ((current = time(NULL)) 0) {
+virReportSystemError(errno, %s,
+ _(failed to get current system time));
+return -1;
+}
+
+localtime_r(current, localtimeinfo);
+gmtime_r(current, gmtimeinfo);
+
+if ((local = mktime(localtimeinfo)) 0 ||
+(utc = mktime(gmtimeinfo)) 0) {
+virReportSystemError(errno, %s,
+ _(mktime failed));
+return -1;
+}
+
+*offset = local - utc;
+if (localtimeinfo.tm_isdst)
+*offset += 3600;
+return 0;
+}
diff --git a/src/util/virtime.h b/src/util/virtime.h
index a5bd652..cda9707 100644
--- a/src/util/virtime.h
+++ b/src/util/virtime.h
@@ -1,7 +1,7 @@
/*
* virtime.h: Time handling functions
*
- * Copyright (C) 2006-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2011, 2014 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -50,7 +50,6 @@ int virTimeStringNowRaw(char *buf)
int virTimeStringThenRaw(unsigned long long when, char *buf)
ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
-
/* These APIs are *not* async signal safe and return -1,
* raising a libvirt error on failure
*/
@@ -63,5 +62,7 @@ int virTimeFieldsThen(unsigned long long when, struct tm
*fields)
char *virTimeStringNow(void);
char *virTimeStringThen(unsigned long long when);
+int virTimeLocalOffsetFromUTC(time_t *offset)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
#endif
diff --git a/tests/virtimetest.c b/tests/virtimetest.c
index 73a3c3e..d1a409f 100644
--- a/tests/virtimetest.c
+++ b/tests/virtimetest.c
@@ -72,6 +72,35 @@ static int testTimeFields(const void *args)
}
+typedef struct {
+const char *zone;
+time_t offset;
+} testTimeLocalOffsetData;
+
+static int
+testTimeLocalOffset(const void *args)
+{
+const testTimeLocalOffsetData *data = args;
+time_t actual;
+
+if (setenv(TZ, data-zone, 1) 0) {
+perror(setenv);
+return -1;
+}
+tzset();
+
+if (virTimeLocalOffsetFromUTC(actual) 0) {
+return -1;
+}
+
+if (data-offset != actual) {
+VIR_DEBUG(Expect Offset %d got %d\n,
+ (int) data-offset, (int) actual);
+return -1;
+}
+return 0;
+}
+
static int
mymain(void)
{
@@ -119,6 +148,21 @@ mymain(void)
TEST_FIELDS(2147483648000ull, 2038, 1, 19, 3, 14, 8);
+#define TEST_LOCALOFFSET(tz, off) \
[libvirt] [PATCHv2 3/4] qemu: fix RTC_CHANGE event for clock offset='variable' basis='utc'/
commit e31b5cf393857 attempted to fix libvirt's
VIR_DOMAIN_EVENT_ID_RTC_CHANGE, which is documentated to always
provide the new offset of the domain's real time clock from UTC. The
problem was that, in the case that qemu is provided with an -rtc
base=x where x is an absolute time (rather than utc or
localtime), the offset sent by qemu's RTC_CHANGE event is *not* the
new offset from UTC, but rather is the sum of all changes to the
domain's RTC since it was started with base=x.
So, despite what was said in commit e31b5cf393857, if we assume that
the original value stored in adjustment was the offset from UTC at
the time the domain was started, we can always determine the current
offset from UTC by simply adding the most recent (i.e. current) offset
from qemu to that original adjustment.
This patch accomplishes that by storing the initial adjustment in the
domain's status as adjustment0. Each time a new RTC_CHANGE event is
received from qemu, we simply add adjustment0 to the value sent by
qemu, store that as the new adjustment, and forward that value on to
any event handler.
This patch (*not* e31b5cf393857, which should be reverted prior to
applying this patch) fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=964177
(for the case where basis='utc'. It does not fix basis='localtime')
---
Changes from V1: remove all attempts to fix basis='localtime' in favor
of fixing it in a simpler and better manner in a separate patch.
src/conf/domain_conf.c | 21 +
src/conf/domain_conf.h | 7 +++
src/qemu/qemu_command.c | 9 +
src/qemu/qemu_process.c | 27 ++-
4 files changed, 55 insertions(+), 9 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a7863c3..cc33b86 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -99,6 +99,7 @@ typedef enum {
VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = (118),
VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM = (119),
VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT = (120),
+ VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST = (121),
} virDomainXMLInternalFlags;
VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
@@ -12002,6 +12003,9 @@ virDomainDefParseXML(xmlDocPtr xml,
if (virXPathLongLong(number(./clock/@adjustment), ctxt,
def-clock.data.variable.adjustment) 0)
def-clock.data.variable.adjustment = 0;
+if (virXPathLongLong(number(./clock/@adjustment0), ctxt,
+ def-clock.data.variable.adjustment0) 0)
+def-clock.data.variable.adjustment0 = 0;
tmp = virXPathString(string(./clock/@basis), ctxt);
if (tmp) {
if ((def-clock.data.variable.basis =
virDomainClockBasisTypeFromString(tmp)) 0) {
@@ -17086,7 +17090,8 @@ virDomainResourceDefFormat(virBufferPtr buf,
verify(((VIR_DOMAIN_XML_INTERNAL_STATUS |
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
- VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES)
+ VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
+ VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST)
DUMPXML_FLAGS) == 0);
/* This internal version can accept VIR_DOMAIN_XML_INTERNAL_*,
@@ -17108,7 +17113,8 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virCheckFlags(DUMPXML_FLAGS |
VIR_DOMAIN_XML_INTERNAL_STATUS |
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
- VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES,
+ VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
+ VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST,
-1);
if (!(type = virDomainVirtTypeToString(def-virtType))) {
@@ -17642,6 +17648,11 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAsprintf(buf, adjustment='%lld' basis='%s',
def-clock.data.variable.adjustment,
virDomainClockBasisTypeToString(def-clock.data.variable.basis));
+if (flags VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST) {
+if (def-clock.data.variable.adjustment0)
+virBufferAsprintf(buf, adjustment0='%lld',
+ def-clock.data.variable.adjustment0);
+}
break;
case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE:
virBufferEscapeString(buf, timezone='%s', def-clock.data.timezone);
@@ -18072,7 +18083,8 @@ virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
unsigned int flags = (VIR_DOMAIN_XML_SECURE |
VIR_DOMAIN_XML_INTERNAL_STATUS |
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
- VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES);
+ VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
+ VIR_DOMAIN_XML_INTERNAL_CLOCK_ADJUST);
int ret = -1;
char *xml;
@@ -18160,7 +18172,8 @@ virDomainObjListLoadStatus(virDomainObjListPtr doms,
if (!(obj = virDomainObjParseFile(statusFile, caps, xmlopt,
[libvirt] [PATCHv2 4/4] qemu: fix clock offset='variable' basis='localtime'/
For a clock element as above, libvirt simply converts current system
time with localtime_r(), then starts qemu with a time string that
doesn't contain any timezone information. So, from qemu's point of
view, the -rtc string it gets for:
clock offset='variable' basis='utc' adjustment='10800'/
is identical to the -rtc string it gets for:
clock offset='variable' basis='localtime' adjustment='0'/
(assuming the host is in a timezone that is 10800 seconds ahead of
UTC, as is the case on the machine where this message is being
written).
Since the commandlines are identical, qemu will behave identically
after this point in either case.
There are two problems in the case of basis='localtime' though:
Problem 1) If the guest modifies its RTC, for example to add 20
seconds, the RTC_CHANGE event from qemu will then contain offset:20 in
both cases. But libvirt will have saved the original adjustment into
adjustment0, and will add that value onto the offset in the
event. This means that in the case of basis=;utc', it will properly
emit an event with offset:10820, but in the case of basis='localtime'
the event will contain offset:20, which is *not* the new offset of the
RTC from UTC (as the event it documented to provide).
Problem 2) If the guest is migrated to another host that is in a
different timezone, or if it is migrated or saved/restored after the
DST status has changed from what it was when the guest was originally
started, the newly restarted guest will have a different RTC (since it
will be based on the new localtime, which could have shifted by
several hours).
The solution to both of these problems is simple - rather than
maintaining the original adjustment value along with
basis='localtime' in the domain status, when the domain is started
we convert the adjustment offset to one relative to UTC, and set the
status to basis='utc'. Thus, whatever the RTC offset was from UTC
when it was initially started, that offset will be maintained when
migrating across timezones and DST settings, and the RTC_CHANGE events
will automatically contain the proper offset (which should by
definition always be relative to UTC).
This fixes a problem that was implied but not openly stated in:
https://bugzilla.redhat.com/show_bug.cgi?id=964177
---
New in V2
src/qemu/qemu_command.c | 33 ++---
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4998bca..28885f2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5976,19 +5976,30 @@ qemuBuildClockArgStr(virDomainClockDefPtr def)
time_t now = time(NULL);
struct tm nowbits;
-switch ((enum virDomainClockBasis) def-data.variable.basis) {
-case VIR_DOMAIN_CLOCK_BASIS_UTC:
-now += def-data.variable.adjustment;
-gmtime_r(now, nowbits);
-break;
-case VIR_DOMAIN_CLOCK_BASIS_LOCALTIME:
-now += def-data.variable.adjustment;
-localtime_r(now, nowbits);
-break;
-case VIR_DOMAIN_CLOCK_BASIS_LAST:
-break;
+if (def-data.variable.basis == VIR_DOMAIN_CLOCK_BASIS_LOCALTIME) {
+time_t localOffset;
+
+/* in the case of basis='localtime', rather than trying to
+ * keep that basis (and associated offset from UTC) in the
+ * status and deal with adding in the difference each time
+ * there is an RTC_CHANGE event, it is simpler and less
+ * error prone to just convert the adjustment an offset
+ * from UTC right now (and change the status to
+ * basis='utc' to reflect this). This eliminates
+ * potential errors in both RTC_CHANGE events and in
+ * migration (in the case that the status of DST, or the
+ * timezone of the destination host, changed relative to
+ * startup).
+ */
+if (virTimeLocalOffsetFromUTC(localOffset) 0)
+ goto error;
+def-data.variable.adjustment += localOffset;
+def-data.variable.basis = VIR_DOMAIN_CLOCK_BASIS_UTC;
}
+now += def-data.variable.adjustment;
+gmtime_r(now, nowbits);
+
/* when an RTC_CHANGE event is received from qemu, we need to
* have the adjustment used at domain start time available to
* compute the new offset from UTC. As this new value is
--
1.9.0
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/4] Revert qemu: Report the offset from host UTC for RTC_CHANGE event
This reverts commit e31b5cf393857a6ca78d148c19393e28dfb39de1.
This commit attempted to work around a bug in the offset value
reported by qemu's RTC_CHANGE event in the case that a variable base
date was given on the qemu commandline. The patch mixed up the math
involved in arriving at the corrected offset to report, and in the
process added an unnecessary private attribute to the clock
element. Since that element is private/internal and not used by anyone
else, it makes sense to simplify things by removing it.
---
Unchanged from V2. This is the direct result of:
git revert e31b5cf393857a6ca78d148c19393e28dfb39de1
src/conf/domain_conf.c | 27 ---
src/conf/domain_conf.h | 5 -
src/qemu/qemu_command.c | 3 ---
src/qemu/qemu_process.c | 13 -
4 files changed, 4 insertions(+), 44 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 055b979..a7863c3 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -99,7 +99,6 @@ typedef enum {
VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES = (118),
VIR_DOMAIN_XML_INTERNAL_ALLOW_ROM = (119),
VIR_DOMAIN_XML_INTERNAL_ALLOW_BOOT = (120),
- VIR_DOMAIN_XML_INTERNAL_BASEDATE = (1 21),
} virDomainXMLInternalFlags;
VIR_ENUM_IMPL(virDomainTaint, VIR_DOMAIN_TAINT_LAST,
@@ -12026,16 +12025,6 @@ virDomainDefParseXML(xmlDocPtr xml,
break;
}
-if (def-clock.offset == VIR_DOMAIN_CLOCK_OFFSET_VARIABLE
-flags VIR_DOMAIN_XML_INTERNAL_BASEDATE) {
-if (virXPathULongLong(number(./clock/@basedate), ctxt,
- def-clock.data.variable.basedate) 0) {
-virReportError(VIR_ERR_XML_ERROR, %s,
- _(invalid basedate));
-goto error;
-}
-}
-
if ((n = virXPathNodeSet(./clock/timer, ctxt, nodes)) 0)
goto error;
@@ -17097,8 +17086,7 @@ virDomainResourceDefFormat(virBufferPtr buf,
verify(((VIR_DOMAIN_XML_INTERNAL_STATUS |
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
- VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
- VIR_DOMAIN_XML_INTERNAL_BASEDATE)
+ VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES)
DUMPXML_FLAGS) == 0);
/* This internal version can accept VIR_DOMAIN_XML_INTERNAL_*,
@@ -17120,8 +17108,7 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virCheckFlags(DUMPXML_FLAGS |
VIR_DOMAIN_XML_INTERNAL_STATUS |
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
- VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
- VIR_DOMAIN_XML_INTERNAL_BASEDATE,
+ VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES,
-1);
if (!(type = virDomainVirtTypeToString(def-virtType))) {
@@ -17655,10 +17642,6 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAsprintf(buf, adjustment='%lld' basis='%s',
def-clock.data.variable.adjustment,
virDomainClockBasisTypeToString(def-clock.data.variable.basis));
-
-if (flags VIR_DOMAIN_XML_INTERNAL_BASEDATE)
-virBufferAsprintf(buf, basedate='%llu',
- def-clock.data.variable.basedate);
break;
case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE:
virBufferEscapeString(buf, timezone='%s', def-clock.data.timezone);
@@ -18089,8 +18072,7 @@ virDomainSaveStatus(virDomainXMLOptionPtr xmlopt,
unsigned int flags = (VIR_DOMAIN_XML_SECURE |
VIR_DOMAIN_XML_INTERNAL_STATUS |
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
- VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
- VIR_DOMAIN_XML_INTERNAL_BASEDATE);
+ VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES);
int ret = -1;
char *xml;
@@ -18178,8 +18160,7 @@ virDomainObjListLoadStatus(virDomainObjListPtr doms,
if (!(obj = virDomainObjParseFile(statusFile, caps, xmlopt,
expectedVirtTypes,
VIR_DOMAIN_XML_INTERNAL_STATUS |
VIR_DOMAIN_XML_INTERNAL_ACTUAL_NET |
- VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES |
- VIR_DOMAIN_XML_INTERNAL_BASEDATE)))
+
VIR_DOMAIN_XML_INTERNAL_PCI_ORIG_STATES)))
goto error;
virUUIDFormat(obj-def-uuid, uuidstr);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index bde303c..94285e3 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1727,11 +1727,6 @@ struct _virDomainClockDef {
struct {
long long adjustment;
int basis;
-
-/* Store the base date (-rtc base=$date, in seconds
- * since the Epoch) of guest process, internal only
- */
-unsigned long long basedate;
} variable;
/* Timezone name, when
diff
Re: [libvirt] [patch v3 2/2] add inotify handler to qemu driver
On Thu, May 22, 2014 at 02:07:32 +, chen.fan.f...@cn.fujitsu.com wrote: On Thu, 2014-05-15 at 11:25 +0200, Jiri Denemark wrote: On Wed, May 07, 2014 at 18:11:50 +0800, Chen Fan wrote: we don't expect to reload 'migrate_uri' with restarting libvirtd everytime while updating the URI, so adding inotify handler to reload 'migrate_uri' configuration without restarting libvirtd, it will be also helpful for virt-manager to get 'migrate_uri'. NACK, if we ever want configuration to be automatically reloaded when configuration file changes (which I seriously doubt, SIGHUP is the standard way of reloading configuration files), we certainly would not want to do it this hacky way and only for one specific option. Not to mention that the content of virQEMUDriverConfig must not change, a completely new structure has to be created with the changed values. Hi Jiri, thanks for reminding me, actually, I knew the virQEMUDriverConfig without lock protect to reload is wrong. except that I think there are some options in virQEMUDriverConfig can be reloaded. as you said, If we want to reload them, Do we must create a new structure to save the changes, then we must use 'lock' to make code thread safe and override the qemuDriverCfg values everywhere while we use them. right? or are there any other ideas? virQEMUDriverConfig is immutable so the structure returned by virQEMUDriverGetConfig(qemu_driver) must never be changed. You have to create a copy of it, update the values in the new structure, take a driver lock, unref the original driver-config and put the new pointer to driver-config. This way, any code that already got virQEMUDriverConfig pointer will be using the old value and all calls to virQEMUDriverGetConfig will return the updated configuration. The old one will be freed when its last user unrefs it. But anyway, we should not be updating the config whenever the configuration file changes. We should rather do that in qemuStateReload. However, I'm not sure what to do if there are options that just cannot be changed while libvirtd is running. Applying some changes while ignoring others when SIGHUP is sent to libvirtd would certainly lead to confusion. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Fix various migration issues
Jiri Denemark (3): Fix error message when TUNNELLED flag is used in non-p2p migration qemu: Send migrate_cancel when aborting migration qemu: Properly abort migration to a file src/libvirt.c | 8 - src/qemu/qemu_migration.c | 74 ++- 2 files changed, 54 insertions(+), 28 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] Fix error message when TUNNELLED flag is used in non-p2p migration
The current error message is
error: use virDomainMigrateToURI3 for peer-to-peer migration
which is correct but a bit misleading because the client did not specify
VIR_MIGRATE_PEER2PEER flag. This patch changes the error message to
error: cannot perform tunnelled migration without using peer2peer
flag
which is consistent with the error reported by older migration APIs.
Reported by Rich Jones in
https://bugzilla.redhat.com/show_bug.cgi?id=1095924
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/libvirt.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 19fa18b..72a9f6d 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5723,12 +5723,18 @@ virDomainMigrate3(virDomainPtr domain,
__FUNCTION__);
goto error;
}
-if (flags (VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_TUNNELLED)) {
+if (flags VIR_MIGRATE_PEER2PEER) {
virReportInvalidArg(flags, %s,
_(use virDomainMigrateToURI3 for peer-to-peer
migration));
goto error;
}
+if (flags VIR_MIGRATE_TUNNELLED) {
+virReportInvalidArg(flags, %s,
+_(cannot perform tunnelled migration
+ without using peer2peer flag));
+goto error;
+}
if (flags VIR_MIGRATE_OFFLINE) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain-conn-driver, domain-conn,
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] qemu: Properly abort migration to a file
This is similar to the previous commit in that we need to explicitly
send migrate_cancel when libvirt detects an error other than those
reported by query-migrate. However, the possibility to hit such error is
pretty small.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_migration.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ae18acb..5754f73 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4704,6 +4704,7 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
int pipeFD[2] = { -1, -1 };
unsigned long saveMigBandwidth = priv-migMaxBandwidth;
char *errbuf = NULL;
+virErrorPtr orig_err = NULL;
/* Increase migration bandwidth to unlimited since target is a file.
* Failure to change migration speed is not fatal. */
@@ -4806,8 +4807,17 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
rc = qemuMigrationWaitForCompletion(driver, vm, asyncJob, NULL, false);
-if (rc 0)
+if (rc 0) {
+if (rc == -2) {
+orig_err = virSaveLastError();
+virCommandAbort(cmd);
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
+qemuMonitorMigrateCancel(priv-mon);
+qemuDomainObjExitMonitor(driver, vm);
+}
+}
goto cleanup;
+}
if (cmd virCommandWait(cmd, NULL) 0)
goto cleanup;
@@ -4815,6 +4825,9 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
ret = 0;
cleanup:
+if (ret 0 !orig_err)
+orig_err = virSaveLastError();
+
/* Restore max migration bandwidth */
if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
qemuMonitorSetMigrationSpeed(priv-mon, saveMigBandwidth);
@@ -4840,6 +4853,12 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
VIR_CGROUP_DEVICE_RWM);
virDomainAuditCgroupPath(vm, priv-cgroup, deny, path, rwm, rv ==
0);
}
+
+if (orig_err) {
+virSetError(orig_err);
+virFreeError(orig_err);
+}
+
return ret;
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] Don't log an internal error when the guest hasn't updated balloon stats
On 05/15/2014 11:19 PM, Eric Blake wrote:
On 05/15/2014 01:22 AM, Ján Tomko wrote:
If virDomainMemoryStats is called too soon after domain startup,
QEMU returns:
error:{class:GenericError,desc:guest hasn't updated any stats yet}
when we try to query balloon stats.
Check for this reply and log it as OPERATION_INVALID instead of
INTERNAL_ERROR. This means the daemon only logs it at the debug level,
without polluting system logs.
Reported by Laszlo Pal:
https://www.redhat.com/archives/libvirt-users/2014-May/msg00023.html
---
v1: https://www.redhat.com/archives/libvir-list/2014-May/msg00420.html
v2:
return 0 in this case - even though balloon stats are not yet available,
we can still return 'rss' in qemuDomainMemoryStats
jump to cleanup if CheckError returns 0
src/qemu/qemu_monitor_json.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
+if ((data = virJSONValueObjectGet(reply, error))) {
+const char *klass = virJSONValueObjectGetString(data, class);
+const char *desc = virJSONValueObjectGetString(data, desc);
-if (ret 0)
+if (STREQ_NULLABLE(klass, GenericError)
+STREQ_NULLABLE(desc, guest hasn't updated any stats yet)) {
Adding qemu. Uggh - the qemu documentation of QMP states:
- The desc member is a human-readable error message. Clients should
not attempt to parse this message.
because the contents of that field are NOT guaranteed to be stable.
We're stuck parsing that field for old versions of qemu, but this is one
case where upstream qemu (for future versions) should change the class
member of that particular error case to a distinct value other than
GenericError so that it is trivially obvious when this particular
condition has occurred, since it is a case where libvirt wants to treat
it as a non-error.
reluctant ACK, while hoping that we can do something more reliable in
the future.
I have pushed the patch now.
The qemu patch reporting empty stats instead of this error should be on its way:
https://lists.gnu.org/archive/html/qemu-devel/2014-05/msg04295.html
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] Host OS, Storage Info
On Thu, May 22, 2014 at 12:16:17PM +0530, Sijo Jose wrote: Hi, Is there any way to get host OS information and host Storage in formations using libvirt API...? Rgds -Sijo Check virsh help (most of the commands you probably want start with 'node' or 'vol'/'pool') or have a look at our hvsupport page [1] for virNode* and virStorage* functions. If this isn't what you're looking for, specify your question in a better way. Martin [1] http://libvirt.org/hvsupport.html signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] PCI: Introduce new device binding path using pci_dev.driver_override
On 05/20/2014 05:53 PM, Alex Williamson wrote: The driver_override field allows us to specify the driver for a device rather than relying on the driver to provide a positive match of the device. This shortcuts the existing process of looking up the vendor and device ID, adding them to the driver new_id, binding the device, then removing the ID, but it also provides a couple advantages. First, the above existing process allows the driver to bind to any device matching the new_id for the window where it's enabled. This is often not desired, such as the case of trying to bind a single device to a meta driver like pci-stub or vfio-pci. Using driver_override we can do this deterministically using: echo pci-stub /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe I'm not qualified to review the mechanics of the patch, but as a potential consumer of this new interface (in libvirt), I want to say that it is *immensely* improved over the racy, error prone method that the kernel currently has available. The first day that I looked at the libvirt code that uses the new_id method to bind drivers to devices, I wished that there would instead be an interface more or less identical to what Alex has described here - simple, deterministic, and with minimal side effect outside of the exact device and driver pair we want to bind. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: Any interest in a weekly(?) dev community meeting ?
While we haven't set any agenda to discuss, I think we should just have a first meeting today regardless just to kickstart the idea. So in 30 mins time on #virt-meeting on irc.oftc.net Daniel On Fri, May 16, 2014 at 01:53:50PM -0400, Daniel P. Berrange wrote: Hi Libvirt team, A number of opensource projects have weekly meetings between their community of contributors to facilitate their day-to-day working and particularly to resolve roadblocks that people are having. I feel that libvirt is large enough, with contributors from many different organizations, that a meeting could be beneficial to our operation. This could serve a number of purposes - Remind us of patches that have been posted and accidentally forgotten by reviewers - Resolve hard debates that are not making adequate progress on the mailing list(s) - Track progress of major ongoing pieces of work with many collaborators - Forum for those new to the community to introduce themselves their ideas before starting work - Discuss release critical bugs during freeze periods - Place for downstream users of libvirt (eg openstack/ovirt/etc) to interact with libvirt team. - Place for projects we use (eg KVM, Xen) to interact with libvirt team. I like to keep the overhead of this low, so I'd suggest we try todo it on IRC, since that has been fairly effective for OpenStack teams. If people think this is worth while I'd suggest an arbitrary time of 1500 UTC using the #virt-meeting IRC channel on irc.oftc.net, to last an absolute max of 1 hour. Currently this time point works out as 08:00 San Francisco 11:00 Boston 15:00 UTC 16:00 London 17:00 Berlin 20:30 Mumbai 23:00 Bejing 24:00 Tokyo http://www.timeanddate.com/worldclock/fixedtime.html?hour=15min=00sec=0p1=0 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 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] [PATCHv2 02/33] qemu: Make qemuDomainPrepareDiskChainElement aware of remote storage
Refactor the function to accept a virStorageSourcePtr instead of just
the path, add a check to run it only on local storage and fix callers
(possibly by using a newly introduced wrapper that wraps a path in the
virStorageSource struct for legacy code)
---
Notes:
Version 2:
- V1 acked by Eric with comment cleanup
src/qemu/qemu_driver.c | 77 --
1 file changed, 49 insertions(+), 28 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6fda50d..d3e14ea 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12020,22 +12020,26 @@ static int
qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
- const char *file,
+ virStorageSourcePtr elem,
qemuDomainDiskChainMode mode)
{
/* The easiest way to label a single file with the same
* permissions it would have as if part of the disk chain is to
* temporarily modify the disk in place. */
-char *origsrc = disk-src.path;
-int origformat = disk-src.format;
-virStorageSourcePtr origchain = disk-src.backingStore;
+virStorageSource origdisk;
bool origreadonly = disk-readonly;
+virQEMUDriverConfigPtr cfg = NULL;
int ret = -1;
-virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver);
-disk-src.path = (char *) file; /* casting away const is safe here */
-disk-src.format = VIR_STORAGE_FILE_RAW;
-disk-src.backingStore = NULL;
+/* XXX Labelling of non-local files isn't currently supported */
+if (virStorageSourceGetActualType(disk-src) == VIR_STORAGE_TYPE_NETWORK)
+return 0;
+
+cfg = virQEMUDriverGetConfig(driver);
+
+/* XXX This would be easier when disk-src will be a pointer */
+memcpy(origdisk, disk-src, sizeof(origdisk));
+memcpy(disk-src, elem, sizeof(*elem));
disk-readonly = mode == VIR_DISK_CHAIN_READ_ONLY;
if (mode == VIR_DISK_CHAIN_NO_ACCESS) {
@@ -12058,9 +12062,7 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr
driver,
ret = 0;
cleanup:
-disk-src.path = origsrc;
-disk-src.format = origformat;
-disk-src.backingStore = origchain;
+memcpy(disk-src, origdisk, sizeof(origdisk));
disk-readonly = origreadonly;
virObjectUnref(cfg);
return ret;
@@ -12071,6 +12073,25 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr
driver,
* is sent but failed, and number of frozen filesystems on success. If -2 is
* returned, FSThaw should be called revert the quiesced status. */
static int
+qemuDomainPrepareDiskChainElementPath(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ virDomainDiskDefPtr disk,
+ const char *file,
+ qemuDomainDiskChainMode mode)
+{
+virStorageSource src;
+
+memset(src, 0, sizeof(src));
+
+src.type = VIR_STORAGE_TYPE_FILE;
+src.format = VIR_STORAGE_FILE_RAW;
+src.path = (char *) file; /* casting away const is safe here */
+
+return qemuDomainPrepareDiskChainElement(driver, vm, disk, src, mode);
+}
+
+
+static int
qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver,
virDomainObjPtr vm,
const char **mountpoints,
@@ -12829,9 +12850,9 @@
qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver,
VIR_FORCE_CLOSE(fd);
}
-if (qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
+if (qemuDomainPrepareDiskChainElement(driver, vm, disk, snap-src,
VIR_DISK_CHAIN_READ_WRITE) 0) {
-qemuDomainPrepareDiskChainElement(driver, vm, disk, source,
+qemuDomainPrepareDiskChainElement(driver, vm, disk, snap-src,
VIR_DISK_CHAIN_NO_ACCESS);
goto cleanup;
}
@@ -12962,7 +12983,7 @@ qemuDomainSnapshotUndoSingleDiskActive(virQEMUDriverPtr
driver,
(persistDisk VIR_STRDUP(persistSource, source) 0))
goto cleanup;
-qemuDomainPrepareDiskChainElement(driver, vm, disk, disk-src.path,
+qemuDomainPrepareDiskChainElement(driver, vm, disk, disk-src,
VIR_DISK_CHAIN_NO_ACCESS);
if (need_unlink
virStorageFileStat(disk-src, st) == 0 S_ISREG(st.st_mode)
@@ -15276,10 +15297,10 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
if (VIR_STRDUP(mirror, dest) 0)
goto endjob;
-if (qemuDomainPrepareDiskChainElement(driver, vm, disk, dest,
- VIR_DISK_CHAIN_READ_WRITE) 0) {
-qemuDomainPrepareDiskChainElement(driver, vm, disk, dest,
-
[libvirt] [PATCHv2 01/33] qemu: process: Refresh backing chain info when reconnecting to qemu
Refresh the disk backing chains when reconnecting to a qemu process after daemon restart. There are a few internal fields that don't get refreshed from the XML. Until we are able to do that, let's reload all the metadata by the backing chain crawler. --- src/qemu/qemu_process.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a83780f..4aa9ca3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3213,6 +3213,11 @@ qemuProcessReconnect(void *opaque) if (qemuTranslateDiskSourcePool(conn, obj-def-disks[i]) 0) goto error; +/* XXX we should be able to restore all data from XML in the future */ +if (qemuDomainDetermineDiskChain(driver, obj, + obj-def-disks[i], true) 0) +goto error; + dev.type = VIR_DOMAIN_DEVICE_DISK; dev.data.disk = obj-def-disks[i]; if (qemuAddSharedDevice(driver, dev, obj-def-name) 0) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 19/33] storage: Change to new backing store parser
Use the new backing store parser in the backing chain crawler. This
change needs one test change where information about the NBD image are
now parsed differently.
---
src/storage/storage_driver.c | 90 +---
tests/virstoragetest.c | 14 ---
2 files changed, 9 insertions(+), 95 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 67882ce..a4518cb 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3068,56 +3068,6 @@ virStorageFileAccess(virStorageSourcePtr src,
}
-/**
- * Given a starting point START (a directory containing the original
- * file, if the original file was opened via a relative path; ignored
- * if NAME is absolute), determine the location of the backing file
- * NAME (possibly relative), and compute the relative DIRECTORY
- * (optional) and CANONICAL (mandatory) location of the backing file.
- * Return 0 on success, negative on error.
- */
-static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
-virFindBackingFile(const char *start, const char *path,
- char **directory, char **canonical)
-{
-/* FIXME - when we eventually allow non-raw network devices, we
- * must ensure that we handle backing files the same way as qemu.
- * For a qcow2 top file of gluster://server/vol/img, qemu treats
- * the relative backing file 'rel' as meaning
- * 'gluster://server/vol/rel', while the backing file '/abs' is
- * used as a local file. But we cannot canonicalize network
- * devices via canonicalize_file_name(), because they are not part
- * of the local file system. */
-char *combined = NULL;
-int ret = -1;
-
-if (*path == '/') {
-/* Safe to cast away const */
-combined = (char *)path;
-} else if (virAsprintf(combined, %s/%s, start, path) 0) {
-goto cleanup;
-}
-
-if (directory !(*directory = mdir_name(combined))) {
-virReportOOMError();
-goto cleanup;
-}
-
-if (!(*canonical = canonicalize_file_name(combined))) {
-virReportSystemError(errno,
- _(Can't canonicalize path '%s'), path);
-goto cleanup;
-}
-
-ret = 0;
-
- cleanup:
-if (combined != path)
-VIR_FREE(combined);
-return ret;
-}
-
-
/* Recursive workhorse for virStorageFileGetMetadata. */
static int
virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
@@ -3126,7 +3076,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
virHashTablePtr cycle)
{
int ret = -1;
-struct stat st;
const char *uniqueName;
char *buf = NULL;
ssize_t headerLen;
@@ -3178,46 +3127,9 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
goto cleanup;
}
-if (VIR_ALLOC(backingStore) 0)
+if (!(backingStore = virStorageSourceNewFromBacking(src)))
goto cleanup;
-if (VIR_STRDUP(backingStore-relPath, src-backingStoreRaw) 0)
-goto cleanup;
-
-if (virStorageIsFile(src-backingStoreRaw)) {
-backingStore-type = VIR_STORAGE_TYPE_FILE;
-
-if (virFindBackingFile(src-relDir,
- src-backingStoreRaw,
- backingStore-relDir,
- backingStore-path) 0) {
-/* the backing file is (currently) unavailable, treat this
- * file as standalone:
- * backingStoreRaw is kept to mark broken image chains */
-VIR_WARN(Backing file '%s' of image '%s' is missing.,
- src-backingStoreRaw, src-path);
-ret = 0;
-goto cleanup;
-}
-
-/* update the type for local storage */
-if (stat(backingStore-path, st) == 0) {
-if (S_ISDIR(st.st_mode)) {
-backingStore-type = VIR_STORAGE_TYPE_DIR;
-backingStore-format = VIR_STORAGE_FILE_DIR;
-} else if (S_ISBLK(st.st_mode)) {
-backingStore-type = VIR_STORAGE_TYPE_BLOCK;
-}
-}
-} else {
-/* TODO: To satisfy the test case, copy the network URI as path. This
- * will be removed later. */
-backingStore-type = VIR_STORAGE_TYPE_NETWORK;
-
-if (VIR_STRDUP(backingStore-path, src-backingStoreRaw) 0)
-goto cleanup;
-}
-
if (backingFormat == VIR_STORAGE_FILE_AUTO !allow_probe)
backingStore-format = VIR_STORAGE_FILE_RAW;
else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 746bf63..29297ef 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -730,20 +730,22 @@ mymain(void)
/* Rewrite qcow2 to use an nbd: protocol as backend */
virCommandFree(cmd);
cmd = virCommandNewArgList(qemuimg, rebase, -u, -f, qcow2,
- -F, raw, -b,
[libvirt] [PATCHv2 00/33] Gluster backing chains and relative block commit/pull
Hi, this is my second take on this topic. Some of the patches are still in a RFC status as the last six patches require qemu support which is still on review upstream. The rest should be fine as is even without the qemu bits. Peter Krempa (33): qemu: process: Refresh backing chain info when reconnecting to qemu qemu: Make qemuDomainPrepareDiskChainElement aware of remote storage storage: Store gluster volume name separately storage: Rework debugging of storage file access through storage driver conf: Fix domain disk path iterator to work with networked storage storage: Add NONE protocol type for network disks storage: Add support for access to files using provided uid/gid storage: Add storage file API to read file headers storage: backend: Add unique id retrieval API storage: Add API to check accessibility of storage volumes storage: Move virStorageFileGetMetadata to the storage driver storage: Determine the local storage type right away test: storage: Initialize storage source to correct type storage: backend: Add possibility to suppress errors from backend lookup storage: Switch metadata crawler to use storage driver to get unique path storage: Switch metadata crawler to use storage driver to read headers storage: Switch metadata crawler to use storage driver file access check storage: Add infrastructure to parse remote network backing names storage: Change to new backing store parser storage: Traverse backing chains of network disks util: string: Return element count from virStringSplit util: string: Add helper to free non-NULL terminated string arrays util: storagefile: Add helper to resolve ../, ./ and in paths util: storage: Add helper to resolve relative path difference util: storagefile: Add canonicalization to virStorageFileSimplifyPath storage: gluster: Add backend to return unique storage file path qemu: json: Add format strings for optional command arguments 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 cfg.mk| 2 +- include/libvirt/libvirt.h.in | 6 + src/Makefile.am | 2 + src/conf/domain_conf.c| 72 ++- src/libvirt_private.syms | 7 +- src/qemu/qemu_capabilities.c | 8 + src/qemu/qemu_capabilities.h | 2 + src/qemu/qemu_command.c | 24 +- src/qemu/qemu_domain.c| 10 +- src/qemu/qemu_driver.c| 160 +-- src/qemu/qemu_migration.c | 6 +- src/qemu/qemu_monitor.c | 21 +- src/qemu/qemu_monitor.h | 4 +- src/qemu/qemu_monitor_json.c | 139 -- src/qemu/qemu_monitor_json.h | 2 + src/qemu/qemu_process.c | 5 + src/security/virt-aa-helper.c | 2 + src/storage/storage_backend.c | 16 +- src/storage/storage_backend.h | 22 +- src/storage/storage_backend_fs.c | 127 +- src/storage/storage_backend_gluster.c | 203 +++-- src/storage/storage_driver.c | 329 +- src/storage/storage_driver.h | 15 +- src/util/virstoragefile.c | 818 -- src/util/virstoragefile.h | 31 +- src/util/virstring.c | 44 +- src/util/virstring.h | 7 + tests/Makefile.am | 7 +- tests/qemumonitorjsontest.c | 2 +- tests/virstoragetest.c| 290 +++- tools/virsh-domain.c | 29 +- 31 files changed, 1988 insertions(+), 424 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 10/33] storage: Add API to check accessibility of storage volumes
Add a storage driver API equivalent ot the access() function.
Implementations for the filesystem and gluster backends are provided.
---
src/storage/storage_backend.h | 5 +
src/storage/storage_backend_fs.c | 13 +
src/storage/storage_backend_gluster.c | 11 +++
src/storage/storage_driver.c | 24
src/storage/storage_driver.h | 1 +
5 files changed, 54 insertions(+)
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 5d71cde..56643fb 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -198,6 +198,10 @@ typedef ssize_t
typedef const char *
(*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src);
+typedef int
+(*virStorageFileBackendAccess)(virStorageSourcePtr src,
+ int mode);
+
virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);
@@ -220,6 +224,7 @@ struct _virStorageFileBackend {
virStorageFileBackendCreate storageFileCreate;
virStorageFileBackendUnlink storageFileUnlink;
virStorageFileBackendStat storageFileStat;
+virStorageFileBackendAccess storageFileAccess;
};
#endif /* __VIR_STORAGE_BACKEND_H__ */
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 1f436fa..85264a7 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1439,6 +1439,15 @@
virStorageFileBackendFileGetUniqueIdentifier(virStorageSourcePtr src)
}
+static int
+virStorageFileBackendFileAccess(virStorageSourcePtr src,
+int mode)
+{
+return virFileAccessibleAs(src-path, mode,
+ src-drv-uid, src-drv-gid);
+}
+
+
virStorageFileBackend virStorageFileBackendFile = {
.type = VIR_STORAGE_TYPE_FILE,
@@ -1448,6 +1457,7 @@ virStorageFileBackend virStorageFileBackendFile = {
.storageFileUnlink = virStorageFileBackendFileUnlink,
.storageFileStat = virStorageFileBackendFileStat,
.storageFileReadHeader = virStorageFileBackendFileReadHeader,
+.storageFileAccess = virStorageFileBackendFileAccess,
.storageFileGetUniqueIdentifier =
virStorageFileBackendFileGetUniqueIdentifier,
};
@@ -1461,6 +1471,7 @@ virStorageFileBackend virStorageFileBackendBlock = {
.storageFileStat = virStorageFileBackendFileStat,
.storageFileReadHeader = virStorageFileBackendFileReadHeader,
+.storageFileAccess = virStorageFileBackendFileAccess,
.storageFileGetUniqueIdentifier =
virStorageFileBackendFileGetUniqueIdentifier,
};
@@ -1472,6 +1483,8 @@ virStorageFileBackend virStorageFileBackendDir = {
.backendInit = virStorageFileBackendFileInit,
.backendDeinit = virStorageFileBackendFileDeinit,
+.storageFileAccess = virStorageFileBackendFileAccess,
+
.storageFileGetUniqueIdentifier =
virStorageFileBackendFileGetUniqueIdentifier,
};
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index badffae..1a844c9 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -703,6 +703,16 @@ virStorageFileBackendGlusterReadHeader(virStorageSourcePtr
src,
}
+static int
+virStorageFileBackendGlusterAccess(virStorageSourcePtr src,
+ int mode)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+
+return glfs_access(priv-vol, src-path, mode);
+}
+
+
virStorageFileBackend virStorageFileBackendGluster = {
.type = VIR_STORAGE_TYPE_NETWORK,
.protocol = VIR_STORAGE_NET_PROTOCOL_GLUSTER,
@@ -713,4 +723,5 @@ virStorageFileBackend virStorageFileBackendGluster = {
.storageFileUnlink = virStorageFileBackendGlusterUnlink,
.storageFileStat = virStorageFileBackendGlusterStat,
.storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
+.storageFileAccess = virStorageFileBackendGlusterAccess,
};
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 2036bfd..cb660ea 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3017,3 +3017,27 @@ virStorageFileGetUniqueIdentifier(virStorageSourcePtr
src)
return src-drv-backend-storageFileGetUniqueIdentifier(src);
}
+
+
+/**
+ * virStorageFileAccess: Check accessability of a storage file
+ *
+ * @src: storage file to check access permissions
+ * @mode: accessibility check options (see man 2 access)
+ *
+ * Returns 0 on success, -1 on error and sets errno. No libvirt
+ * error is reported. Returns -2 if the operation isn't supported
+ * by libvirt storage backend.
+ */
+int
+virStorageFileAccess(virStorageSourcePtr src,
+ int mode)
+{
+if (!virStorageFileIsInitialized(src) ||
+!src-drv-backend-storageFileAccess) {
+errno = ENOSYS;
+return -2;
+}
+
+return src-drv-backend-storageFileAccess(src, mode);
+}
diff --git a/src/storage/storage_driver.h
[libvirt] [PATCHv2 26/33] 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 | 81 +++
1 file changed, 81 insertions(+)
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 1a844c9..25d2aed 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -533,6 +533,7 @@ typedef virStorageFileBackendGlusterPriv
*virStorageFileBackendGlusterPrivPtr;
struct _virStorageFileBackendGlusterPriv {
glfs_t *vol;
+char *uid;
};
@@ -547,6 +548,7 @@ virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)
if (priv-vol)
glfs_fini(priv-vol);
+VIR_FREE(priv-uid);
VIR_FREE(priv);
src-drv-priv = NULL;
@@ -712,6 +714,81 @@ 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 *canonPath = NULL;
+
+if (priv-uid)
+return priv-uid;
+
+if (!(canonPath = virStorageFileSimplifyPathInternal(src-path,
+ false,
+
virStorageFileBackendGlusterReadlinkCallback,
+ priv)))
+return NULL;
+
+ignore_value(virAsprintf(priv-uid, gluster://%s:%s/%s/%s,
+ src-hosts-name,
+ src-hosts-port,
+ src-volume,
+ canonPath));
+
+VIR_FREE(canonPath);
+
+return priv-uid;
+}
+
virStorageFileBackend virStorageFileBackendGluster = {
.type = VIR_STORAGE_TYPE_NETWORK,
@@ -724,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] [PATCHv2 03/33] storage: Store gluster volume name separately
The gluster volume name was previously stored as part of the source path
string. This is unfortunate when we want to do operations on the path as
the volume is used separately.
Parse and store the volume name separately for gluster storage volumes
and use the newly stored variable appropriately.
---
Notes:
Version 2:
- no changes, v1 already ACKed by Eric
src/conf/domain_conf.c| 33 -
src/qemu/qemu_command.c | 19 ++-
src/storage/storage_backend_gluster.c | 27 ---
src/util/virstoragefile.c | 1 +
src/util/virstoragefile.h | 1 +
5 files changed, 56 insertions(+), 25 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 055b979..1ddedac 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5002,6 +5002,27 @@ virDomainDiskSourceParse(xmlNodePtr node,
goto cleanup;
}
+/* for historical reasons the volume name for gluster volume is stored
+ * as a part of the path. This is hard to work with when dealing with
+ * relative names. Split out the volume into a separate variable */
+if (src-path src-protocol == VIR_STORAGE_NET_PROTOCOL_GLUSTER) {
+char *tmp;
+if (!(tmp = strchr(src-path, '/')) ||
+tmp == src-path) {
+virReportError(VIR_ERR_XML_ERROR,
+ _(missing volume name or file name in
+ gluster source path '%s'), src-path);
+goto cleanup;
+}
+
+src-volume = src-path;
+
+if (VIR_STRDUP(src-path, tmp) 0)
+goto cleanup;
+
+tmp[0] = '\0';
+}
+
child = node-children;
while (child != NULL) {
if (child-type == XML_ELEMENT_NODE
@@ -14841,6 +14862,7 @@ virDomainDiskSourceFormat(virBufferPtr buf,
unsigned int flags)
{
size_t n;
+char *path = NULL;
const char *startupPolicy = NULL;
if (policy)
@@ -14876,7 +14898,16 @@ virDomainDiskSourceFormat(virBufferPtr buf,
case VIR_STORAGE_TYPE_NETWORK:
virBufferAsprintf(buf, source protocol='%s',
virStorageNetProtocolTypeToString(src-protocol));
-virBufferEscapeString(buf, name='%s', src-path);
+
+
+if (src-volume) {
+if (virAsprintf(path, %s%s, src-volume, src-path) 0)
+return -1;
+}
+
+virBufferEscapeString(buf, name='%s', path ? path : src-path);
+
+VIR_FREE(path);
if (src-nhosts == 0) {
virBufferAddLit(buf, /\n);
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 193959f..31d0781 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3058,6 +3058,7 @@ qemuNetworkDriveGetPort(int protocol,
static char *
qemuBuildNetworkDriveURI(int protocol,
const char *src,
+ const char *volume,
size_t nhosts,
virStorageNetHostDefPtr hosts,
const char *username,
@@ -3157,11 +3158,18 @@ qemuBuildNetworkDriveURI(int protocol,
if ((uri-port = qemuNetworkDriveGetPort(protocol, hosts-port))
0)
goto cleanup;
-if (src
-virAsprintf(uri-path, %s%s,
-src[0] == '/' ? : /,
-src) 0)
-goto cleanup;
+if (src) {
+if (volume) {
+if (virAsprintf(uri-path, /%s%s,
+volume, src) 0)
+goto cleanup;
+} else {
+if (virAsprintf(uri-path, %s%s,
+src[0] == '/' ? : /,
+src) 0)
+goto cleanup;
+}
+}
if (hosts-socket
virAsprintf(uri-query, socket=%s, hosts-socket) 0)
@@ -3323,6 +3331,7 @@ qemuGetDriveSourceString(virStorageSourcePtr src,
case VIR_STORAGE_TYPE_NETWORK:
if (!(*source = qemuBuildNetworkDriveURI(src-protocol,
src-path,
+ src-volume,
src-nhosts,
src-hosts,
username,
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 2f0592f..8679d96 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -533,8 +533,6 @@ typedef virStorageFileBackendGlusterPriv
[libvirt] [PATCHv2 09/33] storage: backend: Add unique id retrieval API
Different protocols have different means to uniquely identify a storage
file. This patch implements a storage driver API to retrieve a unique
string describing a volume. The current implementation works for local
storage only and returns the canonical path of the volume.
To add caching support the local filesystem driver now has a private
structure holding the cached string, which is created only when it's
initially accessed.
This patch provides the implementation for local files only for start.
---
src/storage/storage_backend.h| 3 +++
src/storage/storage_backend_fs.c | 49
src/storage/storage_driver.c | 30
src/storage/storage_driver.h | 1 +
4 files changed, 83 insertions(+)
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 6be5ca7..5d71cde 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -195,6 +195,8 @@ typedef ssize_t
ssize_t max_len,
char **buf);
+typedef const char *
+(*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src);
virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);
@@ -211,6 +213,7 @@ struct _virStorageFileBackend {
virStorageFileBackendInit backendInit;
virStorageFileBackendDeinit backendDeinit;
virStorageFileBackendReadHeader storageFileReadHeader;
+virStorageFileBackendGetUniqueIdentifier storageFileGetUniqueIdentifier;
/* The following group of callbacks is expected to set errno
* and return -1 on error. No libvirt error shall be reported */
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 33551e7..1f436fa 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1339,6 +1339,14 @@ virStorageBackend virStorageBackendNetFileSystem = {
};
+typedef struct _virStorageFileBackendFsPriv virStorageFileBackendFsPriv;
+typedef virStorageFileBackendFsPriv *virStorageFileBackendFsPrivPtr;
+
+struct _virStorageFileBackendFsPriv {
+char *uid; /* unique file identifier (canonical path) */
+};
+
+
static void
virStorageFileBackendFileDeinit(virStorageSourcePtr src)
{
@@ -1346,16 +1354,27 @@ virStorageFileBackendFileDeinit(virStorageSourcePtr src)
virStorageTypeToString(virStorageSourceGetActualType(src)),
src-path);
+virStorageFileBackendFsPrivPtr priv = src-drv-priv;
+
+VIR_FREE(priv-uid);
+VIR_FREE(priv);
}
static int
virStorageFileBackendFileInit(virStorageSourcePtr src)
{
+virStorageFileBackendFsPrivPtr priv = NULL;
+
VIR_DEBUG(initializing FS storage file %p (%s:%s), src,
virStorageTypeToString(virStorageSourceGetActualType(src)),
src-path);
+if (VIR_ALLOC(priv) 0)
+return -1;
+
+src-drv-priv = priv;
+
return 0;
}
@@ -1403,6 +1422,23 @@ virStorageFileBackendFileReadHeader(virStorageSourcePtr
src,
}
+static const char *
+virStorageFileBackendFileGetUniqueIdentifier(virStorageSourcePtr src)
+{
+virStorageFileBackendFsPrivPtr priv = src-drv-priv;
+
+if (!priv-uid) {
+if (!(priv-uid = canonicalize_file_name(src-path))) {
+virReportSystemError(errno, _(can't canonicalize path '%s'),
+ src-path);
+return NULL;
+}
+}
+
+return priv-uid;
+}
+
+
virStorageFileBackend virStorageFileBackendFile = {
.type = VIR_STORAGE_TYPE_FILE,
@@ -1412,6 +1448,8 @@ virStorageFileBackend virStorageFileBackendFile = {
.storageFileUnlink = virStorageFileBackendFileUnlink,
.storageFileStat = virStorageFileBackendFileStat,
.storageFileReadHeader = virStorageFileBackendFileReadHeader,
+
+.storageFileGetUniqueIdentifier =
virStorageFileBackendFileGetUniqueIdentifier,
};
@@ -1423,7 +1461,18 @@ virStorageFileBackend virStorageFileBackendBlock = {
.storageFileStat = virStorageFileBackendFileStat,
.storageFileReadHeader = virStorageFileBackendFileReadHeader,
+
+.storageFileGetUniqueIdentifier =
virStorageFileBackendFileGetUniqueIdentifier,
};
+virStorageFileBackend virStorageFileBackendDir = {
+.type = VIR_STORAGE_TYPE_DIR,
+
+.backendInit = virStorageFileBackendFileInit,
+.backendDeinit = virStorageFileBackendFileDeinit,
+
+.storageFileGetUniqueIdentifier =
virStorageFileBackendFileGetUniqueIdentifier,
+};
+
#endif /* WITH_STORAGE_FS */
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 3019531..2036bfd 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2987,3 +2987,33 @@ virStorageFileReadHeader(virStorageSourcePtr src,
return ret;
}
+
+
+/*
+ * virStorageFileGetUniqueIdentifier: Get a unique string describing the volume
+ *
+ * @src: file structure pointing to the file
+ *
+ * Returns a string uniquely describing a single volume
[libvirt] [PATCHv2 22/33] 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 ee745ea..cb1fd72 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1901,6 +1901,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 0ab9d96..42d68b5 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] [PATCHv2 08/33] storage: Add storage file API to read file headers
Add storage driver based functions to access headers of storage files
for metadata extraction. Along with this patch a local filesystem and
gluster via libgfapi implementation is provided. The gluster
implementation is based on code of the saferead_lim function.
---
src/storage/storage_backend.h | 7
src/storage/storage_backend_fs.c | 30
src/storage/storage_backend_gluster.c | 66 +++
src/storage/storage_driver.c | 40 +
src/storage/storage_driver.h | 3 ++
5 files changed, 146 insertions(+)
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index fcbb6da..6be5ca7 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -190,6 +190,12 @@ typedef int
(*virStorageFileBackendStat)(virStorageSourcePtr src,
struct stat *st);
+typedef ssize_t
+(*virStorageFileBackendReadHeader)(virStorageSourcePtr src,
+ ssize_t max_len,
+ char **buf);
+
+
virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);
@@ -204,6 +210,7 @@ struct _virStorageFileBackend {
* error on failure. */
virStorageFileBackendInit backendInit;
virStorageFileBackendDeinit backendDeinit;
+virStorageFileBackendReadHeader storageFileReadHeader;
/* The following group of callbacks is expected to set errno
* and return -1 on error. No libvirt error shall be reported */
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index 3d088ba..33551e7 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1375,6 +1375,34 @@ virStorageFileBackendFileStat(virStorageSourcePtr src,
}
+static ssize_t
+virStorageFileBackendFileReadHeader(virStorageSourcePtr src,
+ssize_t max_len,
+char **buf)
+{
+int fd = -1;
+ssize_t ret = -1;
+
+if ((fd = virFileOpenAs(src-path, O_RDONLY, 0,
+src-drv-uid, src-drv-gid, 0)) 0) {
+virReportSystemError(-fd, _(Failed to open file '%s'),
+ src-path);
+return -1;
+}
+
+if ((ret = virFileReadHeaderFD(fd, max_len, buf)) 0) {
+virReportSystemError(errno,
+ _(cannot read header '%s'), src-path);
+goto cleanup;
+}
+
+ cleanup:
+VIR_FORCE_CLOSE(fd);
+
+return ret;
+}
+
+
virStorageFileBackend virStorageFileBackendFile = {
.type = VIR_STORAGE_TYPE_FILE,
@@ -1383,6 +1411,7 @@ virStorageFileBackend virStorageFileBackendFile = {
.storageFileUnlink = virStorageFileBackendFileUnlink,
.storageFileStat = virStorageFileBackendFileStat,
+.storageFileReadHeader = virStorageFileBackendFileReadHeader,
};
@@ -1393,6 +1422,7 @@ virStorageFileBackend virStorageFileBackendBlock = {
.backendDeinit = virStorageFileBackendFileDeinit,
.storageFileStat = virStorageFileBackendFileStat,
+.storageFileReadHeader = virStorageFileBackendFileReadHeader,
};
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index e578e73..badffae 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -638,6 +638,71 @@ virStorageFileBackendGlusterStat(virStorageSourcePtr src,
}
+static ssize_t
+virStorageFileBackendGlusterReadHeader(virStorageSourcePtr src,
+ ssize_t max_len,
+ char **buf)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+glfs_fd_t *fd = NULL;
+size_t alloc = 0;
+size_t size = 0;
+int save_errno;
+ssize_t ret = -1;
+
+*buf = NULL;
+
+if (!(fd = glfs_open(priv-vol, src-path, O_RDONLY))) {
+virReportSystemError(errno, _(Failed to open file '%s'),
+ src-path);
+goto cleanup;
+}
+
+/* code below is shamelesly stolen from saferead_lim */
+for (;;) {
+int count;
+int requested;
+
+if (size + BUFSIZ + 1 alloc) {
+alloc += alloc / 2;
+if (alloc size + BUFSIZ + 1)
+alloc = size + BUFSIZ + 1;
+
+if (VIR_REALLOC_N(*buf, alloc) 0) {
+save_errno = errno;
+break;
+}
+}
+
+/* Ensure that (size + requested = max_len); */
+requested = MIN(size max_len ? max_len - size : 0,
+alloc - size - 1);
+count = glfs_read(fd, *buf + size, requested, 0);
+size += count;
+
+if (count != requested || requested == 0) {
+save_errno = errno;
+if (count 0) {
+virReportSystemError(errno,
+ _(cannot read header '%s'), src-path);
+break;
[libvirt] [PATCHv2 11/33] storage: Move virStorageFileGetMetadata to the storage driver
My future work will modify the metadata crawler function to use the
storage driver file APIs to access the files instead of accessing them
directly so that we will be able to request the metadata for remote
files too. To avoid linking the storage driver to every helper file
using the utils code, the backing chain traversal function needs to be
moved to the storage driver source.
Additionally the virt-aa-helper and virstoragetest programs need to be
linked with the storage driver as a result of this change.
---
cfg.mk| 2 +-
src/Makefile.am | 2 +
src/libvirt_private.syms | 2 +-
src/qemu/qemu_domain.c| 2 +
src/security/virt-aa-helper.c | 2 +
src/storage/storage_driver.c | 233 ++
src/storage/storage_driver.h | 5 +
src/util/virstoragefile.c | 233 +-
src/util/virstoragefile.h | 7 +-
tests/Makefile.am | 7 +-
tests/virstoragetest.c| 2 +
11 files changed, 258 insertions(+), 239 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 5ff2721..9e8fcec 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -774,7 +774,7 @@ sc_prohibit_cross_inclusion:
access/ | conf/) safe=($$dir|conf|util);; \
locking/) safe=($$dir|util|conf|rpc);;\
cpu/| network/| node_device/| rpc/| security/| storage/)\
- safe=($$dir|util|conf);;\
+ safe=($$dir|util|conf|storage);;\
xenapi/ | xenxs/ ) safe=($$dir|util|conf|xen);; \
*) safe=($$dir|$(mid_dirs)|util);;\
esac; \
diff --git a/src/Makefile.am b/src/Makefile.am
index cfb7097..5028f0d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2565,8 +2565,10 @@ virt_aa_helper_LDFLAGS = \
$(PIE_LDFLAGS) \
$(NULL)
virt_aa_helper_LDADD = \
+ libvirt.la \
libvirt_conf.la \
libvirt_util.la \
+ libvirt_driver_storage_impl.la \
../gnulib/lib/libgnu.la
if WITH_DTRACE_PROBES
virt_aa_helper_LDADD += libvirt_probes.lo
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c3332c9..25dab2d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1863,9 +1863,9 @@ virStorageFileFeatureTypeToString;
virStorageFileFormatTypeFromString;
virStorageFileFormatTypeToString;
virStorageFileGetLVMKey;
-virStorageFileGetMetadata;
virStorageFileGetMetadataFromBuf;
virStorageFileGetMetadataFromFD;
+virStorageFileGetMetadataFromFDInternal;
virStorageFileGetSCSIKey;
virStorageFileIsClusterFS;
virStorageFileParseChainIndex;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 78cfdc6..502b7bd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -40,6 +40,8 @@
#include virstoragefile.h
#include virstring.h
+#include storage/storage_driver.h
+
#include sys/time.h
#include fcntl.h
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 32fc04a..bf540b4 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -55,6 +55,8 @@
#include virrandom.h
#include virstring.h
+#include storage/storage_driver.h
+
#define VIR_FROM_THIS VIR_FROM_SECURITY
static char *progname;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index cb660ea..c90107b 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -49,6 +49,7 @@
#include configmake.h
#include virstring.h
#include viraccessapicheck.h
+#include dirname.h
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -3041,3 +3042,235 @@ virStorageFileAccess(virStorageSourcePtr src,
return src-drv-backend-storageFileAccess(src, mode);
}
+
+
+/**
+ * Given a starting point START (a directory containing the original
+ * file, if the original file was opened via a relative path; ignored
+ * if NAME is absolute), determine the location of the backing file
+ * NAME (possibly relative), and compute the relative DIRECTORY
+ * (optional) and CANONICAL (mandatory) location of the backing file.
+ * Return 0 on success, negative on error.
+ */
+static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
+virFindBackingFile(const char *start, const char *path,
+ char **directory, char **canonical)
+{
+/* FIXME - when we eventually allow non-raw network devices, we
+ * must ensure that we handle backing files the same way as qemu.
+ * For a qcow2 top file of gluster://server/vol/img, qemu treats
+ * the relative backing file 'rel' as meaning
+ * 'gluster://server/vol/rel', while
[libvirt] [PATCHv2 14/33] storage: backend: Add possibility to suppress errors from backend lookup
Add a new function wrapper and tweak the storage file backend lookup
function so that it can be used without reporting error. This will be
useful in the metadata crawler code where we need silently break if
metadata retrieval is not supported for the current storage type.
---
src/storage/storage_backend.c | 16 ++--
src/storage/storage_backend.h | 4 +++-
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index e016cc8..380ca58 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -1173,8 +1173,9 @@ virStorageBackendForType(int type)
virStorageFileBackendPtr
-virStorageFileBackendForType(int type,
- int protocol)
+virStorageFileBackendForTypeInternal(int type,
+ int protocol,
+ bool report)
{
size_t i;
@@ -1188,6 +1189,9 @@ virStorageFileBackendForType(int type,
}
}
+if (!report)
+return NULL;
+
if (type == VIR_STORAGE_TYPE_NETWORK) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(missing storage backend for network files
@@ -1203,6 +1207,14 @@ virStorageFileBackendForType(int type,
}
+virStorageFileBackendPtr
+virStorageFileBackendForType(int type,
+ int protocol)
+{
+return virStorageFileBackendForTypeInternal(type, protocol, true);
+}
+
+
struct diskType {
int part_table_type;
unsigned short offset;
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 56643fb..76c1afa 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -203,7 +203,9 @@ typedef int
int mode);
virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol);
-
+virStorageFileBackendPtr virStorageFileBackendForTypeInternal(int type,
+ int protocol,
+ bool report);
struct _virStorageFileBackend {
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 06/33] storage: Add NONE protocol type for network disks
Currently the protocol type with index 0 was NBD which made it hard to
distinguish whether the protocol type was actually assigned. Add a new
protocol type with index 0 to distinguish it explicitly.
---
Notes:
Version 2:
- fixed condition in virDomainDiskSourceParse so that none isn't accepted
- fixed typo in comment in qemuNetworkDriveGetPort()
- fixed commit message
src/conf/domain_conf.c| 2 +-
src/qemu/qemu_command.c | 5 -
src/qemu/qemu_driver.c| 3 +++
src/util/virstoragefile.c | 1 +
src/util/virstoragefile.h | 1 +
5 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4bc71c8..4fb2e1d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4989,7 +4989,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
goto cleanup;
}
-if ((src-protocol = virStorageNetProtocolTypeFromString(protocol))
0){
+if ((src-protocol = virStorageNetProtocolTypeFromString(protocol)) =
0){
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(unknown protocol type '%s'), protocol);
goto cleanup;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 31d0781..0a7b2ff 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3046,7 +3046,8 @@ qemuNetworkDriveGetPort(int protocol,
case VIR_STORAGE_NET_PROTOCOL_RBD:
case VIR_STORAGE_NET_PROTOCOL_LAST:
-/* not aplicable */
+case VIR_STORAGE_NET_PROTOCOL_NONE:
+/* not applicable */
return -1;
}
@@ -3262,6 +3263,7 @@ qemuBuildNetworkDriveURI(int protocol,
case VIR_STORAGE_NET_PROTOCOL_LAST:
+case VIR_STORAGE_NET_PROTOCOL_NONE:
goto cleanup;
}
@@ -11080,6 +11082,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps,
case VIR_STORAGE_NET_PROTOCOL_FTPS:
case VIR_STORAGE_NET_PROTOCOL_TFTP:
case VIR_STORAGE_NET_PROTOCOL_LAST:
+case VIR_STORAGE_NET_PROTOCOL_NONE:
/* ignored for now */
break;
}
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d3e14ea..2b852eb 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12390,6 +12390,7 @@
qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk)
case VIR_STORAGE_TYPE_NETWORK:
switch ((virStorageNetProtocol) disk-src.protocol) {
+case VIR_STORAGE_NET_PROTOCOL_NONE:
case VIR_STORAGE_NET_PROTOCOL_NBD:
case VIR_STORAGE_NET_PROTOCOL_RBD:
case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
@@ -12455,6 +12456,7 @@
qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d
case VIR_STORAGE_NET_PROTOCOL_GLUSTER:
return 0;
+case VIR_STORAGE_NET_PROTOCOL_NONE:
case VIR_STORAGE_NET_PROTOCOL_NBD:
case VIR_STORAGE_NET_PROTOCOL_RBD:
case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
@@ -12597,6 +12599,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr
conn,
case VIR_STORAGE_TYPE_NETWORK:
switch ((virStorageNetProtocol) disk-src.protocol) {
+case VIR_STORAGE_NET_PROTOCOL_NONE:
case VIR_STORAGE_NET_PROTOCOL_NBD:
case VIR_STORAGE_NET_PROTOCOL_RBD:
case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG:
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 856b726..b90cdc9 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -72,6 +72,7 @@ VIR_ENUM_IMPL(virStorageFileFeature,
)
VIR_ENUM_IMPL(virStorageNetProtocol, VIR_STORAGE_NET_PROTOCOL_LAST,
+ none,
nbd,
rbd,
sheepdog,
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index d3560a8..082ff5b 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -119,6 +119,7 @@ struct _virStorageTimestamps {
/* Information related to network storage */
typedef enum {
+VIR_STORAGE_NET_PROTOCOL_NONE,
VIR_STORAGE_NET_PROTOCOL_NBD,
VIR_STORAGE_NET_PROTOCOL_RBD,
VIR_STORAGE_NET_PROTOCOL_SHEEPDOG,
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 16/33] storage: Switch metadata crawler to use storage driver to read headers
Use virStorageFileReadHeader() to read headers of storage files possibly
on remote storate to retrieve the image metadata.
The backend information is now parsed by
virStorageFileGetMetadataInternal which is now exported from the util
source and virStorageFileGetMetadataFromFDInternal now doesn't need to
be exported.
---
src/libvirt_private.syms | 2 +-
src/storage/storage_driver.c | 26 +++---
src/util/virstoragefile.c| 5 ++---
src/util/virstoragefile.h| 9 ++---
4 files changed, 16 insertions(+), 26 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 25dab2d..19c89bc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1865,7 +1865,7 @@ virStorageFileFormatTypeToString;
virStorageFileGetLVMKey;
virStorageFileGetMetadataFromBuf;
virStorageFileGetMetadataFromFD;
-virStorageFileGetMetadataFromFDInternal;
+virStorageFileGetMetadataInternal;
virStorageFileGetSCSIKey;
virStorageFileIsClusterFS;
virStorageFileParseChainIndex;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index eebb7ee..67ab3af 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3132,10 +3132,11 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr
src,
bool allow_probe,
virHashTablePtr cycle)
{
-int fd;
int ret = -1;
struct stat st;
const char *uniqueName;
+char *buf = NULL;
+ssize_t headerLen;
virStorageSourcePtr backingStore = NULL;
int backingFormat;
@@ -3163,26 +3164,13 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr
src,
if (virHashAddEntry(cycle, uniqueName, (void *)1) 0)
goto cleanup;
-if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
-if ((fd = virFileOpenAs(src-path, O_RDONLY, 0, uid, gid, 0)) 0) {
-virReportSystemError(-fd, _(Failed to open file '%s'),
- src-path);
-goto cleanup;
-}
-
-if (virStorageFileGetMetadataFromFDInternal(src, fd,
-backingFormat) 0) {
-VIR_FORCE_CLOSE(fd);
-goto cleanup;
-}
+if ((headerLen = virStorageFileReadHeader(src, VIR_STORAGE_MAX_HEADER,
+ buf)) 0)
+goto cleanup;
-if (VIR_CLOSE(fd) 0)
-VIR_WARN(could not close file %s, src-path);
-} else {
-/* TODO: currently we call this only for local storage */
-ret = 0;
+if (virStorageFileGetMetadataInternal(src, buf, headerLen,
+ backingFormat) 0)
goto cleanup;
-}
/* check whether we need to go deeper */
if (!src-backingStoreRaw) {
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 06bb68c..9f1b4f4 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -740,8 +740,7 @@ qcow2GetFeatures(virBitmapPtr *features,
* with information about the file and its backing store. Return format
* of the backing store as BACKING_FORMAT. PATH and FORMAT have to be
* pre-populated in META */
-static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
-ATTRIBUTE_NONNULL(4)
+int
virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
char *buf,
size_t len,
@@ -962,7 +961,7 @@ virStorageFileGetMetadataFromBuf(const char *path,
/* Internal version that also supports a containing directory name. */
-int
+static int
virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta,
int fd,
int *backingFormat)
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index c1babdc..7dd0187 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -265,9 +265,12 @@ struct _virStorageSource {
int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);
-int virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta,
-int fd,
-int *backingFormat);
+int virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
+ char *buf,
+ size_t len,
+ int *backingFormat)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
+
virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path,
int fd,
int format);
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 05/33] conf: Fix domain disk path iterator to work with networked storage
Skip networked storage but continue iteration through backing chain to
iterate through all the local paths in the backing chain.
---
Notes:
Version 2:
- no change, v1 already ACKed by Eric
src/conf/domain_conf.c | 37 -
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 1ddedac..4bc71c8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18686,34 +18686,37 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
int ret = -1;
size_t depth = 0;
virStorageSourcePtr tmp;
-const char *path = virDomainDiskGetSource(disk);
-int type = virDomainDiskGetType(disk);
+char *brokenRaw = NULL;
-if (!path || type == VIR_STORAGE_TYPE_NETWORK ||
-(type == VIR_STORAGE_TYPE_VOLUME
- disk-src.srcpool
- disk-src.srcpool-mode == VIR_STORAGE_SOURCE_POOL_MODE_DIRECT))
-return 0;
-
-if (iter(disk, path, 0, opaque) 0)
-goto cleanup;
+if (!ignoreOpenFailure) {
+if (virStorageFileChainGetBroken(disk-src, brokenRaw) 0)
+goto cleanup;
-tmp = disk-src.backingStore;
-while (tmp virStorageIsFile(tmp-path)) {
-if (!ignoreOpenFailure tmp-backingStoreRaw !tmp-backingStore) {
+if (brokenRaw) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(unable to visit backing chain file %s),
- tmp-backingStoreRaw);
+ brokenRaw);
goto cleanup;
}
-if (iter(disk, tmp-path, ++depth, opaque) 0)
-goto cleanup;
-tmp = tmp-backingStore;
+}
+
+for (tmp = disk-src; tmp; tmp = tmp-backingStore) {
+int actualType = virStorageSourceGetActualType(tmp);
+/* execute the callback only for local storage */
+if (actualType != VIR_STORAGE_TYPE_NETWORK
+actualType != VIR_STORAGE_TYPE_VOLUME
+tmp-path) {
+if (iter(disk, tmp-path, depth, opaque) 0)
+goto cleanup;
+}
+
+depth++;
}
ret = 0;
cleanup:
+VIR_FREE(brokenRaw);
return ret;
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 04/33] storage: Rework debugging of storage file access through storage driver
Print the debug statements of individual file access functions from the
main API functions instead of the individual backend functions.
Also enhance initialization debug messages on a per-backend basis.
---
Notes:
Version 2:
- no change, v1 already ACKed by Eric
src/storage/storage_backend_fs.c | 43 ++-
src/storage/storage_backend_gluster.c | 30 +---
src/storage/storage_driver.c | 27 +++---
3 files changed, 62 insertions(+), 38 deletions(-)
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index de27890..3d088ba 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -1339,18 +1339,31 @@ virStorageBackend virStorageBackendNetFileSystem = {
};
+static void
+virStorageFileBackendFileDeinit(virStorageSourcePtr src)
+{
+VIR_DEBUG(deinitializing FS storage file %p (%s:%s), src,
+ virStorageTypeToString(virStorageSourceGetActualType(src)),
+ src-path);
+
+}
+
+
static int
-virStorageFileBackendFileUnlink(virStorageSourcePtr src)
+virStorageFileBackendFileInit(virStorageSourcePtr src)
{
-int ret;
+VIR_DEBUG(initializing FS storage file %p (%s:%s), src,
+ virStorageTypeToString(virStorageSourceGetActualType(src)),
+ src-path);
-ret = unlink(src-path);
-/* preserve errno */
+return 0;
+}
-VIR_DEBUG(removing storage file %p(%s): ret=%d, errno=%d,
- src, src-path, ret, errno);
-return ret;
+static int
+virStorageFileBackendFileUnlink(virStorageSourcePtr src)
+{
+return unlink(src-path);
}
@@ -1358,21 +1371,16 @@ static int
virStorageFileBackendFileStat(virStorageSourcePtr src,
struct stat *st)
{
-int ret;
-
-ret = stat(src-path, st);
-/* preserve errno */
-
-VIR_DEBUG(stat of storage file %p(%s): ret=%d, errno=%d,
- src, src-path, ret, errno);
-
-return ret;
+return stat(src-path, st);
}
virStorageFileBackend virStorageFileBackendFile = {
.type = VIR_STORAGE_TYPE_FILE,
+.backendInit = virStorageFileBackendFileInit,
+.backendDeinit = virStorageFileBackendFileDeinit,
+
.storageFileUnlink = virStorageFileBackendFileUnlink,
.storageFileStat = virStorageFileBackendFileStat,
};
@@ -1381,6 +1389,9 @@ virStorageFileBackend virStorageFileBackendFile = {
virStorageFileBackend virStorageFileBackendBlock = {
.type = VIR_STORAGE_TYPE_BLOCK,
+.backendInit = virStorageFileBackendFileInit,
+.backendDeinit = virStorageFileBackendFileDeinit,
+
.storageFileStat = virStorageFileBackendFileStat,
};
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 8679d96..e578e73 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -539,10 +539,12 @@ struct _virStorageFileBackendGlusterPriv {
static void
virStorageFileBackendGlusterDeinit(virStorageSourcePtr src)
{
-VIR_DEBUG(deinitializing gluster storage file %p(%s/%s),
- src, src-hosts[0].name, src-path);
virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+VIR_DEBUG(deinitializing gluster storage file %p (gluster://%s:%s/%s%s),
+ src, src-hosts-name, src-hosts-port ? src-hosts-port : 0,
+ src-volume, src-path);
+
if (priv-vol)
glfs_fini(priv-vol);
@@ -558,12 +560,14 @@ virStorageFileBackendGlusterInit(virStorageSourcePtr src)
const char *hostname = host-name;
int port = 0;
-VIR_DEBUG(initializing gluster storage file %p(%s/%s),
- src, hostname, src-path);
+VIR_DEBUG(initializing gluster storage file %p (gluster://%s:%s/%s%s),
+ src, hostname, host-port ? host-port : 0,
+ NULLSTR(src-volume), src-path);
if (!src-volume) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _(missing gluster volume name for path '%s'),
src-path);
+ _(missing gluster volume name for path '%s'),
+ src-path);
return -1;
}
@@ -619,14 +623,8 @@ static int
virStorageFileBackendGlusterUnlink(virStorageSourcePtr src)
{
virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
-int ret;
-
-ret = glfs_unlink(priv-vol, src-path);
-/* preserve errno */
-VIR_DEBUG(removing storage file %p(%s/%s): ret=%d, errno=%d,
- src, src-hosts[0].name, src-path, ret, errno);
-return ret;
+return glfs_unlink(priv-vol, src-path);
}
@@ -635,14 +633,8 @@ virStorageFileBackendGlusterStat(virStorageSourcePtr src,
struct stat *st)
{
virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
-int ret;
-ret = glfs_stat(priv-vol, src-path, st);
-/* preserve errno */
-
-VIR_DEBUG(stat of storage file %p(%s/%s): ret=%d, errno=%d,
- src,
[libvirt] [PATCHv2 32/33] 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_capabilities.c | 5 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_driver.c | 33 +
3 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 03d8842..ccd9a93 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,
+ block-commit-backing-name,
);
@@ -3100,6 +3101,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
if (virQEMUCapsProbeQMPCommandLine(qemuCaps, mon) 0)
goto cleanup;
+/* TODO, WIP: We shall test this */
+if (qemuCaps-version = 200)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_BLOCKPULL_BACKING_NAME);
+
ret = 0;
cleanup:
VIR_FREE(package);
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index d755caa..773298d 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_BLOCKPULL_BACKING_NAME = 168, /* support for backing name
tweaking */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index e2eccee..1e7ad1a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15414,8 +15414,12 @@ 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;
-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;
@@ -15500,6 +15504,26 @@ 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_BLOCKPULL_BACKING_NAME))
{
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this qemu doesn't support relative blockpull));
+goto endjob;
+}
+
+if (virStorageFileGetRelativeBackingPath(topSource, baseSource,
+ backingPath) 0)
+goto endjob;
+
+/* XXX should we error out? */
+}
+
/* 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
@@ -15507,9 +15531,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);
@@ -15527,6 +15549,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
[libvirt] [PATCHv2 13/33] test: storage: Initialize storage source to correct type
Stat the path of the storage file being tested to set the correct type
into the virStorageSource. This will avoid breaking the test suite when
inquiring metadata of directory paths in the next patches.
---
tests/virstoragetest.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index fb96c71..746bf63 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -98,6 +98,7 @@ testStorageFileGetMetadata(const char *path,
uid_t uid, gid_t gid,
bool allow_probe)
{
+struct stat st;
virStorageSourcePtr ret = NULL;
if (VIR_ALLOC(ret) 0)
@@ -106,6 +107,15 @@ testStorageFileGetMetadata(const char *path,
ret-type = VIR_STORAGE_TYPE_FILE;
ret-format = format;
+if (stat(path, st) == 0) {
+if (S_ISDIR(st.st_mode)) {
+ret-type = VIR_STORAGE_TYPE_DIR;
+ret-format = VIR_STORAGE_FILE_DIR;
+} else if (S_ISBLK(st.st_mode)) {
+ret-type = VIR_STORAGE_TYPE_BLOCK;
+}
+}
+
if (VIR_STRDUP(ret-relPath, path) 0)
goto error;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 18/33] storage: Add infrastructure to parse remote network backing names
Add parsers for relative and absolute backing names for local and remote
storage files.
This parser parses relative paths as relative to their parents and
absolute paths according to the protocol or local access.
For remote storage volumes, all URI based backing file names are
supported and for the qemu colon syntax the NBD protocol is supported.
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 364 ++
src/util/virstoragefile.h | 4 +
3 files changed, 369 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 19c89bc..2b2bc10 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1883,6 +1883,7 @@ virStorageSourceClear;
virStorageSourceClearBackingStore;
virStorageSourceFree;
virStorageSourceGetActualType;
+virStorageSourceNewFromBacking;
virStorageSourcePoolDefFree;
virStorageSourcePoolModeTypeFromString;
virStorageSourcePoolModeTypeToString;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 9f1b4f4..db71cf3 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -38,6 +38,8 @@
#include virendian.h
#include virstring.h
#include virutil.h
+#include viruri.h
+#include dirname.h
#if HAVE_SYS_SYSCALL_H
# include sys/syscall.h
#endif
@@ -660,6 +662,19 @@ virStorageIsFile(const char *backing)
}
+static bool
+virStorageIsRelative(const char *backing)
+{
+if (backing[0] == '/')
+return false;
+
+if (!virStorageIsFile(backing))
+return false;
+
+return true;
+}
+
+
static int
virStorageFileProbeFormatFromBuf(const char *path,
char *buf,
@@ -1560,3 +1575,352 @@ virStorageSourceFree(virStorageSourcePtr def)
virStorageSourceClear(def);
VIR_FREE(def);
}
+
+
+static virStorageSourcePtr
+virStorageSourceNewFromBackingRelative(virStorageSourcePtr parent,
+ const char *rel)
+{
+char *dirname = NULL;
+const char *parentdir = ;
+virStorageSourcePtr ret;
+
+if (VIR_ALLOC(ret) 0)
+return NULL;
+
+ret-backingRelative = true;
+
+/* 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)
+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();
+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;
+
+/* copy the host network part */
+ret-protocol = parent-protocol;
+if (!(ret-hosts = virStorageNetHostDefCopy(parent-nhosts,
parent-hosts)))
+goto error;
+ret-nhosts = parent-nhosts;
+
+if (VIR_STRDUP(ret-volume, parent-volume) 0)
+goto error;
+
+/* XXX store the relative directory name for test's sake */
+if (!(ret-relDir = mdir_name(ret-path))) {
+virReportOOMError();
+goto error;
+}
+}
+
+ cleanup:
+VIR_FREE(dirname);
+return ret;
+
+ error:
+virStorageSourceFree(ret);
+ret = NULL;
+goto cleanup;
+}
+
+
+static int
+virStorageSourceParseBackingURI(virStorageSourcePtr src,
+const char *path)
+{
+virURIPtr uri = NULL;
+char **scheme = NULL;
+int ret = -1;
+
+if (!(uri = virURIParse(path))) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(failed to parse backing file location '%s'),
+ path);
+goto cleanup;
+}
+
+if (!(scheme = virStringSplit(uri-scheme, +, 2)))
+goto cleanup;
+
+if (!scheme[0] ||
+(src-protocol = virStorageNetProtocolTypeFromString(scheme[0])) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(invalid backing protocol '%s'),
+ NULLSTR(scheme[0]));
+goto cleanup;
+}
+
+if (scheme[1]
+(src-hosts-transport =
virStorageNetHostTransportTypeFromString(scheme[1])) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(invalid protocol transport type '%s'),
+ scheme[1]);
+goto cleanup;
+}
+
+/* handle socket stored as a query */
+if (uri-query) {
+if
[libvirt] [PATCHv2 25/33] util: storagefile: Add canonicalization to virStorageFileSimplifyPath
The recently introduced virStorageFileSimplifyPath is good at resolving
relative portions of a path. To add full path canonicalization
capability we need to be able to resolve symlinks in the path too.
This patch adds a callback to that function so that arbitrary storage
systems can use this functionality.
---
src/libvirt_private.syms | 1 +
src/util/virstoragefile.c | 82 +--
src/util/virstoragefile.h | 9 ++
tests/virstoragetest.c| 80 +
4 files changed, 170 insertions(+), 2 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 6787b51..02d86a8 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1872,6 +1872,7 @@ virStorageFileIsClusterFS;
virStorageFileParseChainIndex;
virStorageFileProbeFormat;
virStorageFileResize;
+virStorageFileSimplifyPathInternal;
virStorageIsFile;
virStorageNetHostDefClear;
virStorageNetHostDefCopy;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index a390b1c..330210d 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1959,13 +1959,47 @@ virStorageFileExportPath(char **components,
}
+static int
+virStorageFileExplodePath(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 *
-virStorageFileSimplifyPath(const char *path,
- bool allow_relative)
+virStorageFileSimplifyPathInternal(const char *path,
+ bool allow_relative,
+ virStorageFileSimplifyPathReadlinkCallback
cb,
+ void *cbdata)
{
bool beginSlash = false;
char **components = NULL;
size_t ncomponents = 0;
+char *linkpath = NULL;
+char *currentpath = NULL;
size_t i;
char *ret = NULL;
@@ -2009,6 +2043,40 @@ virStorageFileSimplifyPath(const char *path,
continue;
}
+/* read link and stuff */
+if (cb) {
+int rc;
+if (!(currentpath = virStorageFileExportPath(components, i + 1,
+ beginSlash)))
+goto cleanup;
+
+if ((rc = cb(currentpath, linkpath, cbdata)) 0)
+goto cleanup;
+
+if (rc == 0) {
+if (linkpath[0] == '/') {
+/* start from a clean slate */
+virStringFreeListCount(components, ncomponents);
+ncomponents = 0;
+components = NULL;
+beginSlash = true;
+i = 0;
+} else {
+VIR_FREE(components[i]);
+VIR_DELETE_ELEMENT(components, i, ncomponents);
+}
+
+if (virStorageFileExplodePath(linkpath, i, components,
+ ncomponents) 0)
+goto cleanup;
+
+VIR_FREE(linkpath);
+VIR_FREE(currentpath);
+
+continue;
+}
+}
+
i++;
}
@@ -2016,11 +2084,21 @@ virStorageFileSimplifyPath(const char *path,
cleanup:
virStringFreeListCount(components, ncomponents);
+VIR_FREE(linkpath);
+VIR_FREE(currentpath);
return ret;
}
+char *
+virStorageFileSimplifyPath(const char *path,
+ bool allow_relative)
+{
+return virStorageFileSimplifyPathInternal(path, allow_relative, NULL,
NULL);
+}
+
+
int
virStorageFileGetRelativeBackingPath(virStorageSourcePtr from,
virStorageSourcePtr to,
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 8b23f95..a8412a5 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -324,6 +324,15 @@ void virStorageSourceFree(virStorageSourcePtr def);
void virStorageSourceClearBackingStore(virStorageSourcePtr def);
virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent);
+typedef int
+(*virStorageFileSimplifyPathReadlinkCallback)(const char *path,
+ char **link,
+ void *data);
+char *virStorageFileSimplifyPathInternal(const char *path,
+ bool allow_relative,
+
[libvirt] [PATCHv2 31/33] 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 ++
tools/virsh-domain.c | 22 +++---
2 files changed, 21 insertions(+), 3 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 2df7fc8..a4fe175 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2571,6 +2571,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/tools/virsh-domain.c b/tools/virsh-domain.c
index 5c3a142..74473a8 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 ||
@@ -2071,6 +2075,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}
};
@@ -2091,6 +2100,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;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 24/33] 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 | 45 +
src/util/virstoragefile.h | 4 ++
tests/virstoragetest.c| 101 ++
4 files changed, 151 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index cb1fd72..6787b51 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1866,6 +1866,7 @@ virStorageFileGetLVMKey;
virStorageFileGetMetadataFromBuf;
virStorageFileGetMetadataFromFD;
virStorageFileGetMetadataInternal;
+virStorageFileGetRelativeBackingPath;
virStorageFileGetSCSIKey;
virStorageFileIsClusterFS;
virStorageFileParseChainIndex;
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 21d71c4..a390b1c 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -2019,3 +2019,48 @@ virStorageFileSimplifyPath(const char *path,
return ret;
}
+
+
+int
+virStorageFileGetRelativeBackingPath(virStorageSourcePtr from,
+ virStorageSourcePtr to,
+ char **relpath)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+virStorageSourcePtr next;
+char *tmp = NULL;
+char ret = -1;
+
+*relpath = NULL;
+
+for (next = from; next; next = next-backingStore) {
+if (!next-backingRelative || !next-relPath) {
+ret = 1;
+goto cleanup;
+}
+
+if (next != from)
+virBufferAddLit(buf, /../);
+
+virBufferAdd(buf, next-relPath, -1);
+
+if (next == to)
+break;
+}
+
+if (next != to)
+goto cleanup;
+
+if (!(tmp = virBufferContentAndReset(buf)))
+goto cleanup;
+
+if (!(*relpath = virStorageFileSimplifyPath(tmp, true)))
+goto cleanup;
+
+ret = 0;
+
+ cleanup:
+virBufferFreeAndReset(buf);
+VIR_FREE(tmp);
+return ret;
+}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 3708c5e..8b23f95 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -327,4 +327,8 @@ virStorageSourcePtr
virStorageSourceNewFromBacking(virStorageSourcePtr parent);
char *virStorageFileSimplifyPath(const char *path,
bool allow_relative);
+int virStorageFileGetRelativeBackingPath(virStorageSourcePtr from,
+ virStorageSourcePtr to,
+ char **relpath);
+
#endif /* __VIR_STORAGE_FILE_H__ */
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 57f16ca..80d73ca 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -560,6 +560,85 @@ testPathSimplify(const void *args)
}
+virStorageSource backingchain[9];
+
+static void
+testPathRelativePrepare(void)
+{
+size_t i;
+
+for (i = 0; i ARRAY_CARDINALITY(backingchain) - 1; i++) {
+backingchain[i].backingStore = backingchain[i+1];
+}
+
+backingchain[0].relPath = (char *) /path/to/some/img;
+backingchain[0].backingRelative = false;
+
+backingchain[1].relPath = (char *) asdf;
+backingchain[1].backingRelative = true;
+
+backingchain[2].relPath = (char *) test;
+backingchain[2].backingRelative = true;
+
+backingchain[3].relPath = (char *) blah;
+backingchain[3].backingRelative = true;
+
+backingchain[4].relPath = (char *) /path/to/some/other/img;
+backingchain[4].backingRelative = false;
+
+backingchain[5].relPath = (char *) ../relative/in/other/path;
+backingchain[5].backingRelative = true;
+
+backingchain[6].relPath = (char *) test;
+backingchain[6].backingRelative = true;
+
+backingchain[7].relPath = (char *) ../../../../../below;
+backingchain[7].backingRelative = true;
+
+backingchain[8].relPath = (char *) a/little/more/upwards;
+backingchain[8].backingRelative = true;
+}
+
+
+struct testPathRelativeBacking
+{
+virStorageSourcePtr from;
+virStorageSourcePtr to;
+
+const char *expect;
+};
+
+static int
+testPathRelative(const void *args)
+{
+const struct testPathRelativeBacking *data = args;
+char *actual = NULL;
+int ret = -1;
+
+if (virStorageFileGetRelativeBackingPath(data-from,
+ data-to,
+ actual) 0) {
+fprintf(stderr, relative backing path resolution failed\n);
+goto cleanup;
+}
+
+if (STRNEQ_NULLABLE(data-expect, actual)) {
+fprintf(stderr, relative path resolution from '%s' to '%s':
+expected '%s', got '%s'\n,
+
[libvirt] [PATCHv2 27/33] qemu: json: Add format strings for optional command arguments
This patch adds option to specify that a json qemu command argument is
optional without the need to use if's or ternary operators to pass the
list. Additionally all the modifier characters are documented to avoid
user confusion.
---
src/qemu/qemu_monitor_json.c | 122 +--
1 file changed, 84 insertions(+), 38 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index f8ab975..04249c5 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -451,7 +451,28 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char
*cmdname, ...)
goto error;
}
-/* Keys look like s:name the first letter is a type code */
+/* Keys look like s:name the first letter is a type code:
+ * Explanation of type codes:
+ * s: string value, must be non-null
+ * S: string value, signed, omitted if null
+ *
+ * i: signed integer value
+ * z: signed integer value, omitted if zero
+ *
+ * I: signed long integer value
+ * Z: integer value, signed, omitted if zero
+ *
+ * u: unsigned integer value
+ * p: unsigned integer value, omitted if zero
+ *
+ * U: unsigned long integer value (see below for quirks)
+ * P: unsigned long integer value, omitted if zero,
+ *
+ * d: double precision floating point number
+ * b: boolean value
+ * n: json null value
+ * a: json array
+ */
type = key[0];
key += 2;
@@ -461,9 +482,13 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char
*cmdname, ...)
/* This doesn't support maps, but no command uses those. */
switch (type) {
+case 'S':
case 's': {
char *val = va_arg(args, char *);
if (!val) {
+if (type == 'S')
+continue;
+
virReportError(VIR_ERR_INTERNAL_ERROR,
_(argument key '%s' must not have null value),
key);
@@ -471,18 +496,38 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char
*cmdname, ...)
}
ret = virJSONValueObjectAppendString(jargs, key, val);
} break;
+
+case 'z':
case 'i': {
int val = va_arg(args, int);
+
+if (!val type == 'z')
+continue;
+
ret = virJSONValueObjectAppendNumberInt(jargs, key, val);
} break;
+
+case 'p':
case 'u': {
unsigned int val = va_arg(args, unsigned int);
+
+if (!val type == 'p')
+continue;
+
ret = virJSONValueObjectAppendNumberUint(jargs, key, val);
} break;
+
+case 'Z':
case 'I': {
long long val = va_arg(args, long long);
+
+if (!val type == 'Z')
+continue;
+
ret = virJSONValueObjectAppendNumberLong(jargs, key, val);
} break;
+
+case 'P':
case 'U': {
/* qemu silently truncates numbers larger than LLONG_MAX,
* so passing the full range of unsigned 64 bit integers
@@ -490,23 +535,32 @@ qemuMonitorJSONMakeCommandRaw(bool wrap, const char
*cmdname, ...)
* instead.
*/
long long val = va_arg(args, long long);
+
+if (!val type == 'P')
+continue;
+
ret = virJSONValueObjectAppendNumberLong(jargs, key, val);
} break;
+
case 'd': {
double val = va_arg(args, double);
ret = virJSONValueObjectAppendNumberDouble(jargs, key, val);
} break;
+
case 'b': {
int val = va_arg(args, int);
ret = virJSONValueObjectAppendBoolean(jargs, key, val);
} break;
+
case 'n': {
ret = virJSONValueObjectAppendNull(jargs, key);
} break;
+
case 'a': {
virJSONValuePtr val = va_arg(args, virJSONValuePtr);
ret = virJSONValueObjectAppend(jargs, key, val);
} break;
+
default:
virReportError(VIR_ERR_INTERNAL_ERROR,
_(unsupported data type '%c' for arg '%s'), type,
key - 2);
@@ -2193,19 +2247,14 @@ int qemuMonitorJSONChangeMedia(qemuMonitorPtr mon,
{
int ret;
virJSONValuePtr cmd;
-if (format)
-cmd = qemuMonitorJSONMakeCommand(change,
- s:device, dev_name,
- s:target, newmedia,
- s:arg, format,
- NULL);
-else
-cmd = qemuMonitorJSONMakeCommand(change,
- s:device, dev_name,
- s:target, newmedia,
-
[libvirt] [PATCHv2 20/33] storage: Traverse backing chains of network disks
Now we don't need to skip backing chain detection for remote disks.
---
src/qemu/qemu_domain.c | 8 +++-
src/storage/storage_driver.c | 18 ++
2 files changed, 9 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 502b7bd..1d6f7be 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2428,12 +2428,10 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
int ret = 0;
uid_t uid;
gid_t gid;
-const char *src = virDomainDiskGetSource(disk);
-int type = virDomainDiskGetType(disk);
+int type = virStorageSourceGetActualType(disk-src);
-if (!src ||
-type == VIR_STORAGE_TYPE_NETWORK ||
-type == VIR_STORAGE_TYPE_VOLUME)
+if (type != VIR_STORAGE_TYPE_NETWORK
+!disk-src.path)
goto cleanup;
if (disk-src.backingStore) {
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index a4518cb..9761dd0 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3189,19 +3189,13 @@ virStorageFileGetMetadata(virStorageSourcePtr src,
if (!(cycle = virHashCreate(5, NULL)))
return -1;
-if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
-if (!src-relPath
-VIR_STRDUP(src-relPath, src-path) 0)
-goto cleanup;
+if (!src-relPath
+VIR_STRDUP(src-relPath, src-path) 0)
+goto cleanup;
-if (!src-relDir
-!(src-relDir = mdir_name(src-path))) {
-virReportOOMError();
-goto cleanup;
-}
-} else {
-/* TODO: currently unimplemented for non-local storage */
-ret = 0;
+if (!src-relDir
+!(src-relDir = mdir_name(src-path))) {
+virReportOOMError();
goto cleanup;
}
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 28/33] 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 commad.
---
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 2b852eb..5a4d3b4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15509,6 +15509,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 912bea1..590081c 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=%ld,
+ mon, device, top, base, 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 de724bf..2cac1b3 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 04249c5..5b0766a 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -3441,6 +3441,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;
@@ -3452,6 +3453,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 5dda0ba..c30803f 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 47d7481..e618c67 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,
VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
[libvirt] [PATCHv2 21/33] util: string: Return element count from virStringSplit
To allow using the array manipulation macros on the arrays returned by
virStringSplit we need to know the count of the elements in the array.
Modify virStringSplit to return this value, rename it and add a helper
with the old name so that we don't need to update all the code.
---
src/libvirt_private.syms | 1 +
src/util/virstring.c | 24
src/util/virstring.h | 6 ++
3 files changed, 27 insertions(+), 4 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2b2bc10..ee745ea 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1908,6 +1908,7 @@ virStringSearch;
virStringSortCompare;
virStringSortRevCompare;
virStringSplit;
+virStringSplitCount;
virStrncpy;
virStrndup;
virStrToDouble;
diff --git a/src/util/virstring.c b/src/util/virstring.c
index 7a8430e..6dcc7a8 100644
--- a/src/util/virstring.c
+++ b/src/util/virstring.c
@@ -43,13 +43,15 @@ VIR_LOG_INIT(util.string);
*/
/**
- * virStringSplit:
+ * virStringSplitCount:
* @string: a string to split
* @delim: a string which specifies the places at which to split
* the string. The delimiter is not included in any of the resulting
* strings, unless @max_tokens is reached.
* @max_tokens: the maximum number of pieces to split @string into.
* If this is 0, the string is split completely.
+ * @tokcount: If provided, the value is set to the count of pieces the string
+ *was split to excluding the terminating NULL element.
*
* Splits a string into a maximum of @max_tokens pieces, using the given
* @delim. If @max_tokens is reached, the remainder of @string is
@@ -65,9 +67,11 @@ VIR_LOG_INIT(util.string);
* Return value: a newly-allocated NULL-terminated array of strings. Use
*virStringFreeList() to free it.
*/
-char **virStringSplit(const char *string,
- const char *delim,
- size_t max_tokens)
+char **
+virStringSplitCount(const char *string,
+const char *delim,
+size_t max_tokens,
+size_t *tokcount)
{
char **tokens = NULL;
size_t ntokens = 0;
@@ -109,6 +113,9 @@ char **virStringSplit(const char *string,
goto error;
tokens[ntokens++] = NULL;
+if (tokcount)
+*tokcount = ntokens - 1;
+
return tokens;
error:
@@ -119,6 +126,15 @@ char **virStringSplit(const char *string,
}
+char **
+virStringSplit(const char *string,
+ const char *delim,
+ size_t max_tokens)
+{
+return virStringSplitCount(string, delim, max_tokens, NULL);
+}
+
+
/**
* virStringJoin:
* @strings: a NULL-terminated array of strings to join
diff --git a/src/util/virstring.h b/src/util/virstring.h
index 1ed1046..0ab9d96 100644
--- a/src/util/virstring.h
+++ b/src/util/virstring.h
@@ -26,6 +26,12 @@
# include internal.h
+char **virStringSplitCount(const char *string,
+ const char *delim,
+ size_t max_tokens,
+ size_t *tokcount)
+ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+
char **virStringSplit(const char *string,
const char *delim,
size_t max_tokens)
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 33/33] 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_capabilities.c | 5 -
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_driver.c | 44
3 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index ccd9a93..7c5c989 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -257,6 +257,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
host-pci-multidomain,
msg-timestamp,
block-commit-backing-name,
+ block-stream-backing-name,
);
@@ -3102,8 +3103,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
goto cleanup;
/* TODO, WIP: We shall test this */
-if (qemuCaps-version = 200)
+if (qemuCaps-version = 200) {
virQEMUCapsSet(qemuCaps, QEMU_CAPS_BLOCKPULL_BACKING_NAME);
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_BLOCKSTREAM_BACKING_NAME);
+}
ret = 0;
cleanup:
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 773298d..3938f42 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -207,6 +207,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_HOST_PCI_MULTIDOMAIN = 166, /* support domain 0 in host pci
address */
QEMU_CAPS_MSG_TIMESTAMP = 167, /* -msg timestamp */
QEMU_CAPS_BLOCKPULL_BACKING_NAME = 168, /* support for backing name
tweaking */
+QEMU_CAPS_BLOCKSTREAM_BACKING_NAME = 169, /* support for backing name
tweaking */
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1e7ad1a..19a6763 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14969,6 +14969,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,
@@ -14976,6 +14978,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;
@@ -15037,10 +15046,34 @@ 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_BLOCKSTREAM_BACKING_NAME)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this QEMU binary doesn't support relative
+ block pull/rebase));
+goto endjob;
+}
+
+if (disk-src.backingStore == baseSource) {
+if (baseSource-backingRelative
+VIR_STRDUP(backingPath, baseSource-relPath) 0)
+goto endjob;
+} else {
+if
(virStorageFileGetRelativeBackingPath(disk-src.backingStore,
+ baseSource,
+ backingPath) 0)
+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;
@@ -15110,6 +15143,8 @@ qemuDomainBlockJobImpl(virDomainObjPtr vm,
}
cleanup:
+VIR_FREE(basePath);
+VIR_FREE(backingPath);
VIR_FREE(device);
if (vm)
virObjectUnlock(vm);
@@ -15350,7 +15385,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 |
+
[libvirt] [PATCHv2 12/33] storage: Determine the local storage type right away
When walking the backing chain we previously set the storage type to
_FILE and let the virStorageFileGetMetadataFromFDInternal update it to
the correct type later on.
This patch moves the actual storage type determination to the place
where we parse the backing store name so that the code can later be
switched to use virStorageFileReadHeader() directly.
---
src/storage/storage_driver.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index c90107b..cd4c6b5 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3111,6 +3111,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
{
int fd;
int ret = -1;
+struct stat st;
virStorageSourcePtr backingStore = NULL;
int backingFormat;
@@ -3173,6 +3174,16 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
ret = 0;
goto cleanup;
}
+
+/* update the type for local storage */
+if (stat(backingStore-path, st) == 0) {
+if (S_ISDIR(st.st_mode)) {
+backingStore-type = VIR_STORAGE_TYPE_DIR;
+backingStore-format = VIR_STORAGE_FILE_DIR;
+} else if (S_ISBLK(st.st_mode)) {
+backingStore-type = VIR_STORAGE_TYPE_BLOCK;
+}
+}
} else {
/* TODO: To satisfy the test case, copy the network URI as path. This
* will be removed later. */
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 15/33] storage: Switch metadata crawler to use storage driver to get unique path
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 wether we support a given image is introduced
to avoid errors for unimplemented backends.
---
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 cd4c6b5..eebb7ee 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2788,6 +2788,30 @@ virStorageFileIsInitialized(virStorageSourcePtr src)
return !!src-drv;
}
+
+static bool
+virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
+{
+int actualType = virStorageSourceGetActualType(src);
+virStorageFileBackendPtr backend;
+
+if (!src)
+return false;
+
+if (src-drv) {
+backend = src-drv-backend;
+} else {
+if (!(backend = virStorageFileBackendForTypeInternal(actualType,
+ src-protocol,
+ false)))
+return false;
+}
+
+return backend-storageFileGetUniqueIdentifier
+ backend-storageFileReadHeader
+ backend-storageFileAccess;
+}
+
void
virStorageFileDeinit(virStorageSourcePtr src)
{
@@ -3104,7 +3128,6 @@ virFindBackingFile(const char *start, const char *path,
/* Recursive workhorse for virStorageFileGetMetadata. */
static int
virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
- const char *canonPath,
uid_t uid, gid_t gid,
bool allow_probe,
virHashTablePtr cycle)
@@ -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;
+
+if (virStorageFileInitAs(src, uid, gid) 0)
return -1;
+
+if (!(uniqueName = virStorageFileGetUniqueIdentifier(src)))
+goto cleanup;
+
+if (virHashLookup(cycle, uniqueName)) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(backing store for %s (%s) is self-referential),
+ src-path, uniqueName);
+goto cleanup;
}
-if (virHashAddEntry(cycle, canonPath, (void *)1) 0)
-return -1;
+if (virHashAddEntry(cycle, uniqueName, (void *)1) 0)
+goto cleanup;
if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
if ((fd = virFileOpenAs(src-path, O_RDONLY, 0, uid, gid, 0)) 0) {
virReportSystemError(-fd, _(Failed to open file '%s'),
src-path);
-return -1;
+goto cleanup;
}
if (virStorageFileGetMetadataFromFDInternal(src, fd,
backingFormat) 0) {
VIR_FORCE_CLOSE(fd);
-return -1;
+goto cleanup;
}
if (VIR_CLOSE(fd) 0)
VIR_WARN(could not close file %s, src-path);
} else {
/* TODO: currently we call this only for local storage */
-return 0;
+ret = 0;
+goto cleanup;
}
/* check whether we need to go deeper */
-if (!src-backingStoreRaw)
-return 0;
+if (!src-backingStoreRaw) {
+ret = 0;
+goto cleanup;
+}
if (VIR_ALLOC(backingStore) 0)
-return -1;
+goto cleanup;
if (VIR_STRDUP(backingStore-relPath, src-backingStoreRaw) 0)
goto cleanup;
@@ -3201,7 +3238,6 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
backingStore-format = backingFormat;
if (virStorageFileGetMetadataRecurse(backingStore,
- backingStore-path,
uid, gid, allow_probe,
cycle) 0) {
/* if we fail somewhere midway, just accept and return a
@@ -3215,6 +3251,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
[libvirt] [PATCHv2 17/33] storage: Switch metadata crawler to use storage driver file access check
Use virStorageFileAccess() to to check wether the file is accessible in
the main part of the metadata crawler.
---
src/storage/storage_driver.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 67ab3af..67882ce 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3103,13 +3103,6 @@ virFindBackingFile(const char *start, const char *path,
goto cleanup;
}
-if (virFileAccessibleAs(combined, F_OK, geteuid(), getegid()) 0) {
-virReportSystemError(errno,
- _(Cannot access backing file '%s'),
- combined);
-goto cleanup;
-}
-
if (!(*canonical = canonicalize_file_name(combined))) {
virReportSystemError(errno,
_(Can't canonicalize path '%s'), path);
@@ -3151,6 +3144,13 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
if (virStorageFileInitAs(src, uid, gid) 0)
return -1;
+if (virStorageFileAccess(src, F_OK) 0) {
+virReportSystemError(errno,
+ _(Cannot access backing file %s),
+ src-path);
+goto cleanup;
+}
+
if (!(uniqueName = virStorageFileGetUniqueIdentifier(src)))
goto cleanup;
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 30/33] 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
tools/virsh-domain.c | 7 +++
2 files changed, 11 insertions(+)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 260c971..2df7fc8 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2588,6 +2588,10 @@ typedef enum {
VIR_DOMAIN_BLOCK_COMMIT_DELETE = 1 1, /* Delete any files that are now
invalid after their contents
have been committed */
+VIR_DOMAIN_BLOCK_COMMIT_RELATIVE = 1 2, /* 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/tools/virsh-domain.c b/tools/virsh-domain.c
index 84a6706..5c3a142 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}
};
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 29/33] 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 5a4d3b4..e2eccee 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -14823,7 +14823,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)
@@ -15040,7 +15040,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;
@@ -15080,8 +15080,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 d6271fb..fc166a1 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 590081c..df5dc3a 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] [PATCHv2 23/33] util: storagefile: Add helper to resolve ../, ./ and //// in paths
As gnulib's canonicalize_filename_mode isn't LGPL and relative snapshot
resolution code will need to clean paths with relative path components
this patch adds a libvirt's own implementation of that functionality and
tests to make sure everything works well.
---
src/util/virstoragefile.c | 95 +++
src/util/virstoragefile.h | 2 +
tests/virstoragetest.c| 83 +
3 files changed, 180 insertions(+)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index db71cf3..21d71c4 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
@@ -1924,3 +1925,97 @@ virStorageSourceNewFromBacking(virStorageSourcePtr
parent)
return ret;
}
+
+
+static char *
+virStorageFileExportPath(char **components,
+ size_t ncomponents,
+ bool beginSlash)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+size_t i;
+char *ret = NULL;
+
+if (beginSlash)
+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 ... wel just return an empty string */
+if (!(ret = virBufferContentAndReset(buf)))
+ignore_value(VIR_STRDUP(ret, ));
+
+return ret;
+}
+
+
+char *
+virStorageFileSimplifyPath(const char *path,
+ bool allow_relative)
+{
+bool beginSlash = false;
+char **components = NULL;
+size_t ncomponents = 0;
+size_t i;
+char *ret = NULL;
+
+/* special cases are and //, return them as they are */
+if (STREQ(path, ) || STREQ(path, //)) {
+ignore_value(VIR_STRDUP(ret, path));
+goto cleanup;
+}
+
+if (path[0] == '/')
+beginSlash = true;
+
+if (!(components = virStringSplitCount(path, /, 0, ncomponents)))
+goto cleanup;
+
+i = 0;
+while (i ncomponents) {
+if (STREQ(components[i], ) ||
+STREQ(components[i], .)) {
+VIR_FREE(components[i]);
+VIR_DELETE_ELEMENT(components, i, ncomponents);
+continue;
+}
+
+if (STREQ(components[i], ..)) {
+if (allow_relative !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;
+}
+
+i++;
+}
+
+ret = virStorageFileExportPath(components, ncomponents, beginSlash);
+
+ cleanup:
+virStringFreeListCount(components, ncomponents);
+
+return ret;
+}
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 0d0f958..3708c5e 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -324,5 +324,7 @@ void virStorageSourceFree(virStorageSourcePtr def);
void virStorageSourceClearBackingStore(virStorageSourcePtr def);
virStorageSourcePtr virStorageSourceNewFromBacking(virStorageSourcePtr parent);
+char *virStorageFileSimplifyPath(const char *path,
+ bool allow_relative);
#endif /* __VIR_STORAGE_FILE_H__ */
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index 29297ef..57f16ca 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -512,12 +512,61 @@ testStorageLookup(const void *args)
return ret;
}
+
+struct testPathSimplifyData
+{
+const char *path;
+const char *exp_abs;
+const char *exp_rel;
+};
+
+
+static int
+testPathSimplify(const void *args)
+{
+const struct testPathSimplifyData *data = args;
+char *simple;
+int ret = -1;
+
+if (!(simple = virStorageFileSimplifyPath(data-path, false))) {
+fprintf(stderr, path simplification returned NULL\n);
+goto cleanup;
+}
+
+if (STRNEQ(simple, data-exp_abs)) {
+fprintf(stderr, simplify path (absolute) '%s': expected: '%s', got
'%s'\n,
+data-path, data-exp_abs, simple);
+goto cleanup;
+}
+
+VIR_FREE(simple);
+
+if (!(simple = virStorageFileSimplifyPath(data-path, true))) {
+fprintf(stderr, path simplification returned NULL\n);
+goto cleanup;
+}
+
+if (STRNEQ(simple, data-exp_rel)) {
+fprintf(stderr, simplify path (relative) '%s': expected: '%s', got
'%s'\n,
+data-path, data-exp_rel, simple);
+goto cleanup;
+}
+
+ret =
[libvirt] [PATCHv2 07/33] storage: Add support for access to files using provided uid/gid
To allow using the storage driver APIs to access files on various
storage sources in an universal fashion possibly on storage such as nfs
with root squash we'll need to store the desired uid/gid in the
metadata.
Add new initialisation API that will store the desired uid/gid and a
wrapper for the current use. Additionally add docs for the two APIs.
---
src/storage/storage_backend.h | 3 +++
src/storage/storage_driver.c | 39 ++-
src/storage/storage_driver.h | 5 +++--
3 files changed, 44 insertions(+), 3 deletions(-)
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 456b9d7..fcbb6da 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -169,6 +169,9 @@ typedef virStorageFileBackend *virStorageFileBackendPtr;
struct _virStorageDriverData {
virStorageFileBackendPtr backend;
void *priv;
+
+uid_t uid;
+gid_t gid;
};
typedef int
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 455a2ef..5e740f9 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -2801,13 +2801,37 @@ virStorageFileDeinit(virStorageSourcePtr src)
}
+/**
+ * virStorageFileInitAs:
+ *
+ * @src: storage source definition
+ * @uid: uid to access the file as, -1 for current uid
+ * @gid: gid to access the file as, -1 for current gid
+ *
+ * Initialize a storage source to be used with storage driver. Use the provided
+ * uid and gid if possible for the operations.
+ *
+ * Returns 0 if the storage file was successfully initialized, -1 if the
+ * initialization failed. Libvirt error is reported.
+ */
int
-virStorageFileInit(virStorageSourcePtr src)
+virStorageFileInitAs(virStorageSourcePtr src,
+ uid_t uid, gid_t gid)
{
int actualType = virStorageSourceGetActualType(src);
if (VIR_ALLOC(src-drv) 0)
return -1;
+if (uid == (uid_t) -1)
+src-drv-uid = geteuid();
+else
+src-drv-uid = uid;
+
+if (gid == (gid_t) -1)
+src-drv-gid = getegid();
+else
+src-drv-gid = gid;
+
if (!(src-drv-backend = virStorageFileBackendForType(actualType,
src-protocol)))
goto error;
@@ -2825,6 +2849,19 @@ virStorageFileInit(virStorageSourcePtr src)
/**
+ * virStorageFileInit:
+ *
+ * See virStorageFileInitAs. The file is initialized to be accessed by the
+ * current user.
+ */
+int
+virStorageFileInit(virStorageSourcePtr src)
+{
+return virStorageFileInitAs(src, (uid_t) -1, (gid_t) -1);
+}
+
+
+/**
* virStorageFileCreate: Creates an empty storage file via storage driver
*
* @src: file structure pointing to the file
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index fb03870..49be999 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -29,8 +29,9 @@
# include storage_conf.h
# include virstoragefile.h
-int
-virStorageFileInit(virStorageSourcePtr src);
+int virStorageFileInit(virStorageSourcePtr src);
+int virStorageFileInitAs(virStorageSourcePtr src,
+ uid_t uid, gid_t gid);
void virStorageFileDeinit(virStorageSourcePtr src);
int virStorageFileCreate(virStorageSourcePtr src);
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] Fix error message when TUNNELLED flag is used in non-p2p migration
On 05/22/14 13:55, Jiri Denemark wrote:
The current error message is
error: use virDomainMigrateToURI3 for peer-to-peer migration
which is correct but a bit misleading because the client did not specify
VIR_MIGRATE_PEER2PEER flag. This patch changes the error message to
error: cannot perform tunnelled migration without using peer2peer
flag
which is consistent with the error reported by older migration APIs.
Reported by Rich Jones in
https://bugzilla.redhat.com/show_bug.cgi?id=1095924
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/libvirt.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 19fa18b..72a9f6d 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5723,12 +5723,18 @@ virDomainMigrate3(virDomainPtr domain,
__FUNCTION__);
goto error;
}
-if (flags (VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_TUNNELLED)) {
+if (flags VIR_MIGRATE_PEER2PEER) {
virReportInvalidArg(flags, %s,
_(use virDomainMigrateToURI3 for peer-to-peer
migration));
goto error;
}
+if (flags VIR_MIGRATE_TUNNELLED) {
+virReportInvalidArg(flags, %s,
+_(cannot perform tunnelled migration
+ without using peer2peer flag));
bikeshed This error message will incline the user to specifying the
VIR_MIGRATE_PEER2PEER flag for this api, so that he receives the error
above.
how about use virDomainMigrateToURI3 and specify the
VIR_MIGRATE_PEER2PEER flag for tunelled migration
Not sure whether that will be more clear though.
/bikeshed
+goto error;
+}
if (flags VIR_MIGRATE_OFFLINE) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain-conn-driver, domain-conn,
ACK,
Peter
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] qemu: Send migrate_cancel when aborting migration
On 05/22/14 13:55, Jiri Denemark wrote: When QEMU reports failed or cancelled migration, we don't need to send it migrate_cancel QMP command. But in all other error paths, such as if we detect broken connection to a destination daemon or something else happens inside libvirt, we need to explicitly send migrate_cancel command instead of relying on the migration to be implicitly cancelled when destination QEMU is killed. Because we were not doing so, one could end up with a paused domain after failed migration. https://bugzilla.redhat.com/show_bug.cgi?id=807023 Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/qemu/qemu_migration.c | 53 --- 1 file changed, 27 insertions(+), 26 deletions(-) ACK, Peter 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/3] Fix error message when TUNNELLED flag is used in non-p2p migration
On Thu, May 22, 2014 at 15:56:57 +0200, Peter Krempa wrote:
On 05/22/14 13:55, Jiri Denemark wrote:
The current error message is
error: use virDomainMigrateToURI3 for peer-to-peer migration
which is correct but a bit misleading because the client did not specify
VIR_MIGRATE_PEER2PEER flag. This patch changes the error message to
error: cannot perform tunnelled migration without using peer2peer
flag
which is consistent with the error reported by older migration APIs.
Reported by Rich Jones in
https://bugzilla.redhat.com/show_bug.cgi?id=1095924
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/libvirt.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 19fa18b..72a9f6d 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5723,12 +5723,18 @@ virDomainMigrate3(virDomainPtr domain,
__FUNCTION__);
goto error;
}
-if (flags (VIR_MIGRATE_PEER2PEER | VIR_MIGRATE_TUNNELLED)) {
+if (flags VIR_MIGRATE_PEER2PEER) {
virReportInvalidArg(flags, %s,
_(use virDomainMigrateToURI3 for peer-to-peer
migration));
goto error;
}
+if (flags VIR_MIGRATE_TUNNELLED) {
+virReportInvalidArg(flags, %s,
+_(cannot perform tunnelled migration
+ without using peer2peer flag));
bikeshed This error message will incline the user to specifying the
VIR_MIGRATE_PEER2PEER flag for this api, so that he receives the error
above.
how about use virDomainMigrateToURI3 and specify the
VIR_MIGRATE_PEER2PEER flag for tunelled migration
Not sure whether that will be more clear though.
/bikeshed
Well, the API documentation says
VIR_MIGRATE_TUNNELLED and VIR_MIGRATE_PEER2PEER are not supported by
this API, use virDomainMigrateToURI3 instead.
so any direct user of this API should already call the right API. And
virsh users would just see the new error and once they add the peer2peer
flag, virsh will automatically use the correct API.
So I guess it's good enough :-)
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] qemu: Properly abort migration to a file
On 05/22/14 13:55, Jiri Denemark wrote:
This is similar to the previous commit in that we need to explicitly
send migrate_cancel when libvirt detects an error other than those
reported by query-migrate. However, the possibility to hit such error is
pretty small.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_migration.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ae18acb..5754f73 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4704,6 +4704,7 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
int pipeFD[2] = { -1, -1 };
unsigned long saveMigBandwidth = priv-migMaxBandwidth;
char *errbuf = NULL;
+virErrorPtr orig_err = NULL;
/* Increase migration bandwidth to unlimited since target is a file.
* Failure to change migration speed is not fatal. */
@@ -4806,8 +4807,17 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
rc = qemuMigrationWaitForCompletion(driver, vm, asyncJob, NULL, false);
-if (rc 0)
+if (rc 0) {
+if (rc == -2) {
+orig_err = virSaveLastError();
+virCommandAbort(cmd);
Shouldn't we abort the migration in qemu before killing the I/O helper?
Qemu will get a sigpipe probably after we do that. Not sure wether
that's worth dealing with as the migration is still going to fail.
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0) {
+qemuMonitorMigrateCancel(priv-mon);
+qemuDomainObjExitMonitor(driver, vm);
+}
+}
goto cleanup;
+}
if (cmd virCommandWait(cmd, NULL) 0)
goto cleanup;
ACK, preferably you respond to my question before pushing :)
Peter
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 3/3] qemu: Properly abort migration to a file
On Thu, May 22, 2014 at 16:14:22 +0200, Peter Krempa wrote:
On 05/22/14 13:55, Jiri Denemark wrote:
This is similar to the previous commit in that we need to explicitly
send migrate_cancel when libvirt detects an error other than those
reported by query-migrate. However, the possibility to hit such error is
pretty small.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_migration.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ae18acb..5754f73 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4704,6 +4704,7 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
int pipeFD[2] = { -1, -1 };
unsigned long saveMigBandwidth = priv-migMaxBandwidth;
char *errbuf = NULL;
+virErrorPtr orig_err = NULL;
/* Increase migration bandwidth to unlimited since target is a file.
* Failure to change migration speed is not fatal. */
@@ -4806,8 +4807,17 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
rc = qemuMigrationWaitForCompletion(driver, vm, asyncJob, NULL, false);
-if (rc 0)
+if (rc 0) {
+if (rc == -2) {
+orig_err = virSaveLastError();
+virCommandAbort(cmd);
Shouldn't we abort the migration in qemu before killing the I/O helper?
Qemu will get a sigpipe probably after we do that. Not sure wether
that's worth dealing with as the migration is still going to fail.
Hmm, good question. Do we even need to abort I/O helper as I guess
cancelling the migration will result in EOF on the I/O helper's input?
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0)
{
+qemuMonitorMigrateCancel(priv-mon);
+qemuDomainObjExitMonitor(driver, vm);
+}
+}
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Weekly meeting minutes May 22nd, 2014
Agenda + minutes from 22/05/2014 * Welcome / rollcall * IRC comments from: * eblake * danpb * pkrempa * derRichard * rbogorodskiy * mkletzan * nehaljwani * mprivozn * Next release schedule * Freeze: Mon May 26th * Release: Mon June 2nd * Patches needing review * pkrempa's gluster patches * 1-27 are nice to have but not release critical * 28+ need to wait for QEMU work * eblake's live commit work * minimum need patch to avoid hang with qemu 2.0 * remaining patches should rebase on top of pkrempa, even if it misses release * vbox snapshot patches - been waiting a long time * Jim Fehlig has libxl migration patches - probably desired for the release * Roman has bhyve pci allocation patches * Refactoring of PCI stuff already merged, just bhyve specific bits left * Nehal has lease helper patch pending * eblake to update to latest gnulib pre-freeze, unless the gnulib change from STREQ macro to inline function streq() requires lots of fallout * Michael has Keep original file label left over from March, not release critical though * Richard W suggests to add support for capabilties for LXC * Allows apps to be started with less caps (ie drop CAP_NET_ADMIN to prevent IP changes) * Could also allow uid/gid to be set Thanks to all for attending this first meeting. Set to repeat again on May 29th @ 1400 UTC in #virt-meeting on irc.oftc.net 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] qemu: Properly abort migration to a file
On 05/22/14 16:28, Jiri Denemark wrote:
On Thu, May 22, 2014 at 16:14:22 +0200, Peter Krempa wrote:
On 05/22/14 13:55, Jiri Denemark wrote:
This is similar to the previous commit in that we need to explicitly
send migrate_cancel when libvirt detects an error other than those
reported by query-migrate. However, the possibility to hit such error is
pretty small.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_migration.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ae18acb..5754f73 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4704,6 +4704,7 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
int pipeFD[2] = { -1, -1 };
unsigned long saveMigBandwidth = priv-migMaxBandwidth;
char *errbuf = NULL;
+virErrorPtr orig_err = NULL;
/* Increase migration bandwidth to unlimited since target is a file.
* Failure to change migration speed is not fatal. */
@@ -4806,8 +4807,17 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
rc = qemuMigrationWaitForCompletion(driver, vm, asyncJob, NULL, false);
-if (rc 0)
+if (rc 0) {
+if (rc == -2) {
+orig_err = virSaveLastError();
+virCommandAbort(cmd);
Shouldn't we abort the migration in qemu before killing the I/O helper?
Qemu will get a sigpipe probably after we do that. Not sure wether
that's worth dealing with as the migration is still going to fail.
Hmm, good question. Do we even need to abort I/O helper as I guess
cancelling the migration will result in EOF on the I/O helper's input?
Well and a counter-example is if qemu get's stuck, then you at least
abort the iohelper and then get stuck a few lines below on the monitor.
+if (qemuDomainObjEnterMonitorAsync(driver, vm, asyncJob) == 0)
{
+qemuMonitorMigrateCancel(priv-mon);
+qemuDomainObjExitMonitor(driver, vm);
+}
+}
Jirka
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-users] Host OS, Storage Info
Sijo, *Pertaining to virt-manager and virsh sluggish behaviour after a clone operation:* Thanks for your response. Honestly I do not know what host Storage in formations using libvirt API means. Sorry. I use virt-manager and virsh to do everything within KVM. If there is something better or another product/app that will enable me to drill down into the system...let me know... However, perhaps this can help: We are running *CentOS 6 (Update 5) 64 bit* - patched as of 11 April 2014. I create the virtual machines with the virt-install command using the* --file* switch and lay the system images of the vm's on the RAID5. The RAID5 uses ext4. The I/O to that volume is nice. We currently are running twenty-six (26) VM's. There is no I/O wait. The system has been up for thirteen (13) days. The load index (top) is between 1 and 3. Also, I have the following kernel tweak in /etc/sysctl.conf: vm.drop_caches = 3 NOTE: Writing to this will cause the kernel to drop clean caches, dentries and inodes from memory, causing that memory to become free; this helps to mitigate dipping into swap. Thanks in advance for everything, Tom On Thu, May 22, 2014 at 2:46 AM, Sijo Jose mailtosijoj...@gmail.com wrote: Hi, Is there any way to get host OS information and host Storage in formations using libvirt API...? Rgds -Sijo ___ libvirt-users mailing list libvirt-us...@redhat.com https://www.redhat.com/mailman/listinfo/libvirt-users -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: Fix specifying char devs for PPC
[meta-comment] On 05/22/2014 04:55 AM, Olivia Yin wrote: Your message came with deep threading (each message in the series in reply to the previous). Observe what happens to replies which are threaded as typical in most mail clients (all replies to a given message sorted by time received) - the replies get interleaved to the wrong patch: 0 + 1 | + 2 | + Re: 1 | + 3 | + Re: 2 | + Re: 3 + Re: 0 whereas we usually prefer shallow threading (the git send-email default, each message in the series in reply only to the cover letter). This time, the replies are easier to spot: 0 + 1 | + Re: 1 + 2 | + Re: 2 + 3 | + Re: 3 Re: 0 -- 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] conf: fix backing store parse off-by-one
On 05/22/2014 01:22 AM, Jiri Denemark wrote: On Wed, May 21, 2014 at 22:06:08 -0600, Eric Blake wrote: Commit 546154e parses the type attribute from a backingStore element, but forgot that the earlier commit 9673418 added a placeholder element in the same 1.2.3 release; as a result, the C code was mistakenly allowing none as a type. Similarly, the same commit allows none as the format sub-element type, even though that has been a placeholder since the 0.10.2 release with commit f772b3d. * src/conf/domain_conf.c (virDomainDiskBackingStoreParse): Require non-zero types. Signed-off-by: Eric Blake ebl...@redhat.com --- Maybe worth addressing in a later patch: the RelaxNG grammar currently requires a format and backingStore subelement to any non-empty backingStore, and the C code matches this requirement. However, we should probably make both of them optional, to represent the case where the user is requesting that we perform a probe to complete the backing chain details. Right, however, this change will only make sense when we actually add support for user-supplied backing chains so implementing this part earlier is useless and perhaps even confusing. Good, we're on the same page about delaying that for another patch. ACK The off-by-one fixes are now 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] [PATCHv2 1/4] util: new function virTimeLocalOffsetFromUTC
On 05/22/2014 05:07 AM, Laine Stump wrote: Since there isn't a single libc API to get this value, this patch supplies one which gets the value by grabbing current UTC, then converting that into a struct tm with localtime_r(), then back to a time_t using mktime; it again does the same operation, but using gmtime_r() instead (for UTC). It then subtracts utc time from the localtime, and finally adjusts if dst is set in the localtime timeinfo (because for some reason mktime doesn't take that into account). This function should be POSIX-compliant, and is threadsafe, but not async signal safe. If it was ever necessary to know this value in a child process, we could cache it with a one-time init function when libvirtd starts, then just supply the cached value, but that complexity isn't needed for current usage. --- Change from V1: add test cases with TZ set to different values (if someone knows how to force DST on/off, I would gladly add some test cases for this as well). Re-reading POSIX: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html I think you can do something like: VIR00:30VID where VIR is the normal time, and VID is the daylight-savings time 1 hour ahead. It was the absence of a second name that made POSIX treat the TZ value as not having daylight savings. I'll review the actual patch in another mail. -- 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 3/3] qemu: Properly abort migration to a file
On Thu, May 22, 2014 at 18:33:08 +0200, Peter Krempa wrote:
On 05/22/14 16:28, Jiri Denemark wrote:
On Thu, May 22, 2014 at 16:14:22 +0200, Peter Krempa wrote:
On 05/22/14 13:55, Jiri Denemark wrote:
This is similar to the previous commit in that we need to explicitly
send migrate_cancel when libvirt detects an error other than those
reported by query-migrate. However, the possibility to hit such error is
pretty small.
Signed-off-by: Jiri Denemark jdene...@redhat.com
---
src/qemu/qemu_migration.c | 21 -
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ae18acb..5754f73 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -4704,6 +4704,7 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
int pipeFD[2] = { -1, -1 };
unsigned long saveMigBandwidth = priv-migMaxBandwidth;
char *errbuf = NULL;
+virErrorPtr orig_err = NULL;
/* Increase migration bandwidth to unlimited since target is a file.
* Failure to change migration speed is not fatal. */
@@ -4806,8 +4807,17 @@ qemuMigrationToFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
rc = qemuMigrationWaitForCompletion(driver, vm, asyncJob, NULL,
false);
-if (rc 0)
+if (rc 0) {
+if (rc == -2) {
+orig_err = virSaveLastError();
+virCommandAbort(cmd);
Shouldn't we abort the migration in qemu before killing the I/O helper?
Qemu will get a sigpipe probably after we do that. Not sure wether
that's worth dealing with as the migration is still going to fail.
Hmm, good question. Do we even need to abort I/O helper as I guess
cancelling the migration will result in EOF on the I/O helper's input?
Well and a counter-example is if qemu get's stuck, then you at least
abort the iohelper and then get stuck a few lines below on the monitor.
Fair enough, I'll push the patch as-is. We don't really care if QEMU
aborts migration after the I/O helper is killed since we're going to
cancel it anyway.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
This introduces two new attributes cmd_per_lun and max_sectors same
with the names QEMU uses for virtio-scsi. An example of the XML:
controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50'
max_sectors='512'/
The corresponding QEMU command line:
-device virtio-scsi-pci,id=scsi0,cmd_per_lun=50,max_sectors=512,
bus=pci.0,addr=0x3
Signed-off-by: Mike Perez thin...@gmail.com
---
docs/formatdomain.html.in | 29 +++---
docs/schemas/domaincommon.rng | 10
src/conf/domain_conf.c | 27 ++--
src/conf/domain_conf.h | 2 ++
src/qemu/qemu_command.c| 27
.../qemuxml2argv-disk-virtio-scsi-cmd_per_lun.args | 9 +++
.../qemuxml2argv-disk-virtio-scsi-cmd_per_lun.xml | 29 ++
.../qemuxml2argv-disk-virtio-scsi-max_sectors.args | 9 +++
.../qemuxml2argv-disk-virtio-scsi-max_sectors.xml | 29 ++
tests/qemuxml2argvtest.c | 6 +
tests/qemuxml2xmltest.c| 2 ++
11 files changed, 168 insertions(+), 11 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-cmd_per_lun.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-cmd_per_lun.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-max_sectors.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-max_sectors.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 691a451..8ffb247 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2559,11 +2559,32 @@
p
An optional sub-element codedriver/code can specify the driver
- specific options. Currently it only supports attribute
codequeues/code
- (span class=since1.0.5/span, QEMU and KVM only), which specifies
the
- number of queues for the controller. For best performance, it's
recommended
- to specify a value matching the number of vCPUs.
+ specific options:
/p
+dl
+ dtcodequeues/code/dt
+ dd
+The optional codequeues/code attribute specifies the number of
+queues for the controller. For best performance, it's recommended to
+specify a value matching the number of vCPUs.
+span class=sinceSince 1.0.5 (QEMU and KVM only)/span
+ /dd
+ dtcodecmd_per_lun/code/dt
+ dd
+The optional codecmd_per_lun/code attribute specifies the maximum
+number of commands that can be queued on devices controlled by the
+host.
+span class=sinceSince 1.2.5 (QEMU and KVM only)/span
+ /dd
+ dtcodemax_sectors/code/dt
+ dd
+The optional codemax_sectors/code attribute specifies the maximum
+amount of data in bytes that will be transferred to or from the device
+in a single command. The transfer length is measured in sectors, where
+a sector is 512 bytes.
+span class=sinceSince 1.2.5 (QEMU and KVM only)/span
+ /dd
+/dl
p
USB companion controllers have an optional
sub-element codelt;mastergt;/code to specify the exact
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4249ed5..4bf4699 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1735,6 +1735,16 @@
ref name=unsignedInt/
/attribute
/optional
+ optional
+ attribute name=cmd_per_lun
+ref name=unsignedInt/
+ /attribute
+/optional
+optional
+ attribute name=max_sectors
+ref name=unsignedInt/
+ /attribute
+/optional
/element
/optional
/interleave
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 40c385e..4388cb8 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6041,6 +6041,8 @@ virDomainControllerDefParseXML(xmlNodePtr node,
char *idx = NULL;
char *model = NULL;
char *queues = NULL;
+char *cmd_per_lun = NULL;
+char *max_sectors = NULL;
xmlNodePtr saved = ctxt-node;
int rc;
@@ -6084,6 +6086,8 @@ virDomainControllerDefParseXML(xmlNodePtr node,
if (cur-type == XML_ELEMENT_NODE) {
if (xmlStrEqual(cur-name, BAD_CAST driver))
queues = virXMLPropString(cur, queues);
+cmd_per_lun = virXMLPropString(cur, cmd_per_lun);
+max_sectors = virXMLPropString(cur, max_sectors);
}
cur = cur-next;
}
@@ -6094,6 +6098,17 @@ virDomainControllerDefParseXML(xmlNodePtr node,
goto error;
}
+if (cmd_per_lun virStrToLong_ui(cmd_per_lun, NULL, 10,
def-cmd_per_lun) 0) {
+virReportError(VIR_ERR_XML_ERROR,
+
Re: [libvirt] [PATCHv2 1/4] util: new function virTimeLocalOffsetFromUTC
On 05/22/2014 05:07 AM, Laine Stump wrote:
Since there isn't a single libc API to get this value, this patch
supplies one which gets the value by grabbing current UTC, then
converting that into a struct tm with localtime_r(), then back to a
time_t using mktime; it again does the same operation, but using
gmtime_r() instead (for UTC). It then subtracts utc time from the
localtime, and finally adjusts if dst is set in the localtime timeinfo
(because for some reason mktime doesn't take that into account).
This function should be POSIX-compliant, and is threadsafe, but not
async signal safe. If it was ever necessary to know this value in a
child process, we could cache it with a one-time init function when
libvirtd starts, then just supply the cached value, but that
complexity isn't needed for current usage.
---
+
+/**
+ * virTimeLocalOffsetFromUTC:
+ *
+ * This function is threadsafe, but is *not* async signal safe (due to
+ * localtime_r()).
+ *
+ * @offset: pointer to time_t that will difference between localtime
s/will/will be set to the/
+ * and UTC in seconds.
Should you also mention whether positive is east of UTC vs. west of UTC?
time_t is not necessarily a signed type, but even if it is unsigned,
you can treat it as a 2s-complement negative value. Maybe this function
should take 'long *offset' instead of 'time_t *offset', to match the
POSIX 'timezone' variable?
+ *
+ * Returns 0 on success, -1 on error with error reported
+ */
+int
+virTimeLocalOffsetFromUTC(time_t *offset)
+{
+struct tm gmtimeinfo, localtimeinfo;
+time_t current, utc, local;
+
+if ((current = time(NULL)) 0) {
time_t is allowed to be an unsigned type. The only portable way to
check for failure is:
if ((current = time(NULL)) == (time_t)-1) {
+virReportSystemError(errno, %s,
+ _(failed to get current system time));
+return -1;
+}
+
+localtime_r(current, localtimeinfo);
+gmtime_r(current, gmtimeinfo);
localtime_r() and gmtime_r() can fail (returning NULL, and leaving
unspecified contents in the 2nd argument) - although arguably the
failure can only occur due to EOVERFLOW at the year 2038 boundary. So
ignoring the error is not necessarily fatal, although it might be worth
a command and/or ignore_value() to state why we are ignoring failure.
+
+if ((local = mktime(localtimeinfo)) 0 ||
+(utc = mktime(gmtimeinfo)) 0) {
Two more comparisons that must be made against ==(time_t)-1, rather than
0, due to time_t possibly being unsigned. What's more, mktime() will
fail only for EOVERFLOW; but if that's the case, then the earlier
localtime_r() or even time() calls would have failed. It seems
inconsistent to check here but not earlier.
+virReportSystemError(errno, %s,
+ _(mktime failed));
+return -1;
+}
+
+*offset = local - utc;
+if (localtimeinfo.tm_isdst)
+*offset += 3600;
Technically, POSIX allows for a daylight savings that is different than
a mere 3600-second gap. But I'm not sure how you would be able to
figure that out from struct tm.
It would be a LOT simpler to just do:
#include time.h
tzset();
*offset = timezone;
except that some older builds of mingw lack the extern variable
timezone. Or maybe even do a configure check, for
AC_CHECK_DECLS([timezone]) (untested, just throwing out the idea), and
having #if HAVE_DECL_TIMEZONE with the short code and the #else clause
using this dance as the fallback for mingw? Or even just ditch older
mingw? I see this in Fedora 20's cross-packages for mingw:
/usr/i686-w64-mingw32/sys-root/mingw/include/time.h: __MINGW_IMPORT
long _timezone;
/usr/i686-w64-mingw32/sys-root/mingw/include/time.h: _CRTIMP extern
long timezone;
but have no idea how accurate they are.
@@ -119,6 +148,21 @@ mymain(void)
TEST_FIELDS(2147483648000ull, 2038, 1, 19, 3, 14, 8);
+#define TEST_LOCALOFFSET(tz, off) \
+do {\
+ testTimeLocalOffsetData data = { \
+ .zone = tz, \
+ .offset = off, \
+}; \
+if (virtTestRun(Test localtime offset for #tz, \
+ testTimeLocalOffset, data) 0) \
+ret = -1; \
+} while (0)
+
+TEST_LOCALOFFSET(VIR00:30, -30 * 60);
+TEST_LOCALOFFSET(UTC, 0);
+TEST_LOCALOFFSET(VIR-00:30, 30 * 60);
This looks accurate, but as you say, it doesn't do any testing of
daylight savings to know if we're getting that part right.
--
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] [PATCHv2 2/4] Revert qemu: Report the offset from host UTC for RTC_CHANGE event
On 05/22/2014 05:07 AM, Laine Stump wrote: This reverts commit e31b5cf393857a6ca78d148c19393e28dfb39de1. This commit attempted to work around a bug in the offset value reported by qemu's RTC_CHANGE event in the case that a variable base date was given on the qemu commandline. The patch mixed up the math involved in arriving at the corrected offset to report, and in the process added an unnecessary private attribute to the clock element. Since that element is private/internal and not used by anyone else, it makes sense to simplify things by removing it. --- Unchanged from V2. This is the direct result of: git revert e31b5cf393857a6ca78d148c19393e28dfb39de1 src/conf/domain_conf.c | 27 --- src/conf/domain_conf.h | 5 - src/qemu/qemu_command.c | 3 --- src/qemu/qemu_process.c | 13 - 4 files changed, 4 insertions(+), 44 deletions(-) ACK; but don't push until we have the rest of the series in good shape. -- 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] [PATCHv2 1/4] util: new function virTimeLocalOffsetFromUTC
On Thu, May 22, 2014 at 02:07:27PM +0300, Laine Stump wrote: Since there isn't a single libc API to get this value, this patch supplies one which gets the value by grabbing current UTC, then converting that into a struct tm with localtime_r(), then back to a time_t using mktime; it again does the same operation, but using gmtime_r() instead (for UTC). It then subtracts utc time from the localtime, and finally adjusts if dst is set in the localtime timeinfo (because for some reason mktime doesn't take that into account). This function should be POSIX-compliant, and is threadsafe, but not async signal safe. If it was ever necessary to know this value in a child process, we could cache it with a one-time init function when libvirtd starts, then just supply the cached value, but that complexity isn't needed for current usage. --- Change from V1: add test cases with TZ set to different values (if someone knows how to force DST on/off, I would gladly add some test cases for this as well). man tzset: The second format is used when there is daylight saving time: std offset dst [offset],start[/time],end[/time] -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 3/4] qemu: fix RTC_CHANGE event for clock offset='variable' basis='utc'/
[Adding qemu] On 05/22/2014 05:07 AM, Laine Stump wrote: commit e31b5cf393857 attempted to fix libvirt's VIR_DOMAIN_EVENT_ID_RTC_CHANGE, which is documentated to always s/documentated/documented/ provide the new offset of the domain's real time clock from UTC. The problem was that, in the case that qemu is provided with an -rtc base=x where x is an absolute time (rather than utc or localtime), the offset sent by qemu's RTC_CHANGE event is *not* the new offset from UTC, but rather is the sum of all changes to the domain's RTC since it was started with base=x. So, despite what was said in commit e31b5cf393857, if we assume that the original value stored in adjustment was the offset from UTC at the time the domain was started, we can always determine the current offset from UTC by simply adding the most recent (i.e. current) offset from qemu to that original adjustment. Is this true even if we miss an RTC update event from qemu? I'm worried about the following situation: user prepares to do a libvirtd upgrade, so libvirtd is shut down. Then the guest triggers an RTC update, so qemu sends an event, but the event is lost. Then libvirtd starts again, and doesn't realize the event is lost. Do we need more help from qemu, such as a new field to an existing QMP command (or a new QMP command) that lists the cumulative offset that qemu is using, where we call that query command any time after an RTC update event or after a libvirtd restart? I'm wondering if this is more a bug in qemu for not providing the right information rather than libvirt's responsibility to work around it. If the only way to keep accurate information is to sum the values we get from events, we are at risk of a lost event getting us messed up. This patch accomplishes that by storing the initial adjustment in the domain's status as adjustment0. Each time a new RTC_CHANGE event is received from qemu, we simply add adjustment0 to the value sent by qemu, store that as the new adjustment, and forward that value on to any event handler. This patch (*not* e31b5cf393857, which should be reverted prior to applying this patch) fixes: https://bugzilla.redhat.com/show_bug.cgi?id=964177 (for the case where basis='utc'. It does not fix basis='localtime') --- Changes from V1: remove all attempts to fix basis='localtime' in favor of fixing it in a simpler and better manner in a separate patch. I'd also appreciate it if the qemu developers can chime in on what is supposed to happen for localtime guests. There are at least four combinations: host running on UTC bios time, guest running on UTC time (in my opinion, the only sane setting, but we're talking about reality not sanity) host running on UTC, guest running on localtime (perhaps the guest is windows, and we know that windows prefers to run on localtime) host running on localtime bios (perhaps because it is dual-boot with windows, and windows prefers bios in localtime), guest running on UTC time host running on localtime, guest running on localtime But it gets even more complicated. The host localtime need not be consistent with the guest localtime. That is, I could be a cloud provider with servers on the east coast, and renting out processor time to a client on the west coast that wants their guest tied to west coast localtime. And that's assuming that both host and guest switch in and out of daylight savings at the same time, which falls apart when you cross political boundaries. Then there's the fun of migration (what if my server farm is spread across multiple timezones - does migration take into account the difference in localtime between source and destination servers). I can _totally_ understand the desire to run a GUEST in such a way that the guest thinks it has a bios stored in localtime (and when the guest updates the RTC twice a year to account for daylight savings, it changes what offset we track about the guest). But I think it is INSANITY to ever try and run a host on a localtime system (daylight savings changes in the host are just asking for problems to the guests) - so even if the host is tied to localtime bios, it is still probably wiser for qemu to base its offsets to UTC no matter what. If the commandline allows a specification of a localtime offset, I think it should be used ONLY for a one-time up-front conversion into a corresponding UTC offset, and then execute qemu in relation to utc thereafter (therefore, migration is always done in terms of utc, without regards for whether source and destination have a different localtime). src/conf/domain_conf.c | 21 + src/conf/domain_conf.h | 7 +++ src/qemu/qemu_command.c | 9 + src/qemu/qemu_process.c | 27 ++- 4 files changed, 55 insertions(+), 9 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c -if (vm-def-clock.offset == VIR_DOMAIN_CLOCK_OFFSET_VARIABLE) +if
Re: [libvirt] [PATCHv2 4/4] qemu: fix clock offset='variable' basis='localtime'/
On 05/22/2014 05:07 AM, Laine Stump wrote:
For a clock element as above, libvirt simply converts current system
time with localtime_r(), then starts qemu with a time string that
doesn't contain any timezone information. So, from qemu's point of
view, the -rtc string it gets for:
clock offset='variable' basis='utc' adjustment='10800'/
is identical to the -rtc string it gets for:
clock offset='variable' basis='localtime' adjustment='0'/
(assuming the host is in a timezone that is 10800 seconds ahead of
UTC, as is the case on the machine where this message is being
written).
Since the commandlines are identical, qemu will behave identically
after this point in either case.
So basically, we are arguing that basis='localtime' should be treated as
syntactic sugar which we rewrite to a more convenient form at the time
we start the guest, and not as a permanent basis that we attempt to
migrate. I can live with that.
There are two problems in the case of basis='localtime' though:
Problem 1) If the guest modifies its RTC, for example to add 20
seconds, the RTC_CHANGE event from qemu will then contain offset:20 in
both cases. But libvirt will have saved the original adjustment into
adjustment0, and will add that value onto the offset in the
event. This means that in the case of basis=;utc', it will properly
emit an event with offset:10820, but in the case of basis='localtime'
the event will contain offset:20, which is *not* the new offset of the
RTC from UTC (as the event it documented to provide).
Problem 2) If the guest is migrated to another host that is in a
different timezone, or if it is migrated or saved/restored after the
DST status has changed from what it was when the guest was originally
started, the newly restarted guest will have a different RTC (since it
will be based on the new localtime, which could have shifted by
several hours).
The solution to both of these problems is simple - rather than
maintaining the original adjustment value along with
basis='localtime' in the domain status, when the domain is started
we convert the adjustment offset to one relative to UTC, and set the
status to basis='utc'. Thus, whatever the RTC offset was from UTC
when it was initially started, that offset will be maintained when
migrating across timezones and DST settings, and the RTC_CHANGE events
will automatically contain the proper offset (which should by
definition always be relative to UTC).
Okay, so this patch is cementing some of the arguments I made in
response to 3/4 (before I had read this patch).
This fixes a problem that was implied but not openly stated in:
https://bugzilla.redhat.com/show_bug.cgi?id=964177
---
New in V2
src/qemu/qemu_command.c | 33 ++---
1 file changed, 22 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 4998bca..28885f2 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -5976,19 +5976,30 @@ qemuBuildClockArgStr(virDomainClockDefPtr def)
time_t now = time(NULL);
struct tm nowbits;
-switch ((enum virDomainClockBasis) def-data.variable.basis) {
-case VIR_DOMAIN_CLOCK_BASIS_UTC:
-now += def-data.variable.adjustment;
-gmtime_r(now, nowbits);
-break;
-case VIR_DOMAIN_CLOCK_BASIS_LOCALTIME:
-now += def-data.variable.adjustment;
-localtime_r(now, nowbits);
-break;
-case VIR_DOMAIN_CLOCK_BASIS_LAST:
-break;
+if (def-data.variable.basis == VIR_DOMAIN_CLOCK_BASIS_LOCALTIME) {
+time_t localOffset;
+
+/* in the case of basis='localtime', rather than trying to
+ * keep that basis (and associated offset from UTC) in the
+ * status and deal with adding in the difference each time
+ * there is an RTC_CHANGE event, it is simpler and less
+ * error prone to just convert the adjustment an offset
s/an/to an/
+ * from UTC right now (and change the status to
+ * basis='utc' to reflect this). This eliminates
+ * potential errors in both RTC_CHANGE events and in
+ * migration (in the case that the status of DST, or the
+ * timezone of the destination host, changed relative to
+ * startup).
including migration to file, across a single host that changes localtime
RTC across daylight savings between when the file was saved and when it
gets loaded :)
+ */
+if (virTimeLocalOffsetFromUTC(localOffset) 0)
+ goto error;
+def-data.variable.adjustment += localOffset;
+def-data.variable.basis = VIR_DOMAIN_CLOCK_BASIS_UTC;
}
+now += def-data.variable.adjustment;
+gmtime_r(now, nowbits);
Seems to make sense. But I'd like feedback
Re: [libvirt] [PATCHv2 01/33] qemu: process: Refresh backing chain info when reconnecting to qemu
On 05/22/2014 07:47 AM, Peter Krempa wrote: Refresh the disk backing chains when reconnecting to a qemu process after daemon restart. There are a few internal fields that don't get refreshed from the XML. Until we are able to do that, let's reload all the metadata by the backing chain crawler. Not necessarily ideal. Reading metadata from a file that is also simultaneously opened read-write by qemu might, in super-rare circumstances, hit a race where qemu is in the middle of updating metadata due to the completion of some job that was started before the libvirtd restart. Although qcow2 has a hard cap of header fitting in one cluster (typically 64k) (at least, after CVE-2014-0144, qemu.git commit 24342f2ca), it is still plausible that qemu is updating the qcow2 header via sectors (perhaps as small as 512-byte chunks) rather than by full clusters; and there are scenarios where an update touches two non-adjacent sectors (such as adjusting the backing file name), such that if a reader sees the old content of one sector but the new content of the other, then it is completely broken in interpreting the data. A slightly safer alternative might be using a QMP command to ask qemu what the backing chain currently is when we reconnect - although that may require correlation back to filenames we passed in, since in the face of fd passing, qemu may only know the name /dev/fdset/NNN instead of the filename the earlier libvirtd opened and passed in by fd. And that presupposes that qemu can always be trusted (we don't want to introduce a CVE where a guest that compromises qemu into returning bogus QMP results can then cause libvirt to mismanage the host filesystem). So with those points made, a metadata read race is extremely rare, and I see no point in implementing a half-way solution that requires parsing a QMP command if we are eventually going to switch over to full XML tracking anyways. Therefore, since _this_ patch is a clear improvement (even if not theoretically perfect), we shouldn't hold it up waiting for some other solution. ACK as-is. src/qemu/qemu_process.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a83780f..4aa9ca3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -3213,6 +3213,11 @@ qemuProcessReconnect(void *opaque) if (qemuTranslateDiskSourcePool(conn, obj-def-disks[i]) 0) goto error; +/* XXX we should be able to restore all data from XML in the future */ and thanks for the comment to remind us what we should improve. +if (qemuDomainDetermineDiskChain(driver, obj, + obj-def-disks[i], true) 0) +goto error; + dev.type = VIR_DOMAIN_DEVICE_DISK; dev.data.disk = obj-def-disks[i]; if (qemuAddSharedDevice(driver, dev, obj-def-name) 0) -- 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] [PATCHv2 01/33] qemu: process: Refresh backing chain info when reconnecting to qemu
On 05/22/2014 03:20 PM, Eric Blake wrote: On 05/22/2014 07:47 AM, Peter Krempa wrote: Refresh the disk backing chains when reconnecting to a qemu process after daemon restart. There are a few internal fields that don't get refreshed from the XML. Until we are able to do that, let's reload all the metadata by the backing chain crawler. Not necessarily ideal. Reading metadata from a file that is also simultaneously opened read-write by qemu might, in super-rare circumstances, hit a race where qemu is in the middle of updating metadata due to the completion of some job that was started before the libvirtd restart. For that matter, all of our other places where we reread the backing chain while qemu is still in operation are also suspect, so your patch isn't making it any worse :) ACK as-is. -- 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] [PATCHv2 02/33] qemu: Make qemuDomainPrepareDiskChainElement aware of remote storage
On 05/22/2014 07:47 AM, Peter Krempa wrote:
Refactor the function to accept a virStorageSourcePtr instead of just
the path, add a check to run it only on local storage and fix callers
(possibly by using a newly introduced wrapper that wraps a path in the
virStorageSource struct for legacy code)
---
Notes:
Version 2:
- V1 acked by Eric with comment cleanup
ACK still holds.
This is a large series; you should go ahead and push ACK'd patches
(especially if it is before the 1.2.5 freeze) rather than waiting for a
full-series review, unless stated explicitly in a patch, or until I hit
something that requires rework that might affect later patches. [that,
and I have patches for live commit that I'm waiting to rebase on top of
yours...]
qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver,
virDomainObjPtr vm,
virDomainDiskDefPtr disk,
- const char *file,
+ virStorageSourcePtr elem,
qemuDomainDiskChainMode mode)
{
/* The easiest way to label a single file with the same
* permissions it would have as if part of the disk chain is to
* temporarily modify the disk in place. */
-char *origsrc = disk-src.path;
-int origformat = disk-src.format;
-virStorageSourcePtr origchain = disk-src.backingStore;
+virStorageSource origdisk;
+
+/* XXX This would be easier when disk-src will be a pointer */
+memcpy(origdisk, disk-src, sizeof(origdisk));
Yep, this is definitely going to conflict with my proposed patch for
converting to virStorageSourcePtr in domain_conf.h. I don't mind
rebasing on top of yours (in part because the compiler will enforce that
my mechanical fallout for a type change catches all uses), but it does
mean yours has to go in first :)
--
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] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
On 05/22/2014 12:22 PM, Mike Perez wrote:
This introduces two new attributes cmd_per_lun and max_sectors same
with the names QEMU uses for virtio-scsi. An example of the XML:
controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50'
max_sectors='512'/
I'm not a fan of underscore (why type the shift key, when a dash will
do). The libvirt xml name does not have to exactly match the qemu
option name, so maybe there's some room for bikeshedding what names we
should actually present to the user.
The corresponding QEMU command line:
-device virtio-scsi-pci,id=scsi0,cmd_per_lun=50,max_sectors=512,
bus=pci.0,addr=0x3
Signed-off-by: Mike Perez thin...@gmail.com
---
docs/formatdomain.html.in | 29
+++---
docs/schemas/domaincommon.rng | 10
src/conf/domain_conf.c | 27 ++--
src/conf/domain_conf.h | 2 ++
src/qemu/qemu_command.c| 27
.../qemuxml2argv-disk-virtio-scsi-cmd_per_lun.args | 9 +++
.../qemuxml2argv-disk-virtio-scsi-cmd_per_lun.xml | 29
++
.../qemuxml2argv-disk-virtio-scsi-max_sectors.args | 9 +++
.../qemuxml2argv-disk-virtio-scsi-max_sectors.xml | 29
++
tests/qemuxml2argvtest.c | 6 +
tests/qemuxml2xmltest.c| 2 ++
Good job covering docs and testsuite additions alongside the code changes.
+++ b/docs/schemas/domaincommon.rng
@@ -1735,6 +1735,16 @@
ref name=unsignedInt/
/attribute
/optional
+ optional
TAB damage.
@@ -15279,13 +15296,19 @@ virDomainControllerDefFormat(virBufferPtr buf,
break;
}
-if (def-queues || virDomainDeviceInfoIsSet(def-info, flags) ||
-pcihole64) {
+if (def-queues || def-cmd_per_lun || def-max_sectors ||
+ virDomainDeviceInfoIsSet(def-info, flags) || pcihole64) {
More TAB damage. Please make sure 'make syntax-check' passes.
@@ -4369,6 +4380,12 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
if (def-queues)
virBufferAsprintf(buf, ,num_queues=%u, def-queues);
+if (def-cmd_per_lun)
+ virBufferAsprintf(buf, ,cmd_per_lun=%u, def-cmd_per_lun);
and again
But once we settle on the naming, the patch looks pretty good.
--
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] [PATCHv2 03/33] storage: Store gluster volume name separately
On 05/22/2014 07:47 AM, Peter Krempa wrote: The gluster volume name was previously stored as part of the source path string. This is unfortunate when we want to do operations on the path as the volume is used separately. Parse and store the volume name separately for gluster storage volumes and use the newly stored variable appropriately. --- Notes: Version 2: - no changes, v1 already ACKed by Eric ACK still holds. -- 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] [PATCHv2 06/33] storage: Add NONE protocol type for network disks
On 05/22/2014 07:47 AM, Peter Krempa wrote:
Currently the protocol type with index 0 was NBD which made it hard to
distinguish whether the protocol type was actually assigned. Add a new
protocol type with index 0 to distinguish it explicitly.
---
Notes:
Version 2:
- fixed condition in virDomainDiskSourceParse so that none isn't
accepted
- fixed typo in comment in qemuNetworkDriveGetPort()
- fixed commit message
src/conf/domain_conf.c| 2 +-
src/qemu/qemu_command.c | 5 -
src/qemu/qemu_driver.c| 3 +++
src/util/virstoragefile.c | 1 +
src/util/virstoragefile.h | 1 +
5 files changed, 10 insertions(+), 2 deletions(-)
ACK with one more nit.
Previous ack on 4 and 5 still stands, too.
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4bc71c8..4fb2e1d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4989,7 +4989,7 @@ virDomainDiskSourceParse(xmlNodePtr node,
goto cleanup;
}
-if ((src-protocol = virStorageNetProtocolTypeFromString(protocol))
0){
+if ((src-protocol = virStorageNetProtocolTypeFromString(protocol))
= 0){
Worth adding the space before { as long as you are touching this line.
--
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] [PATCHv2 07/33] storage: Add support for access to files using provided uid/gid
On 05/22/2014 07:47 AM, Peter Krempa wrote:
To allow using the storage driver APIs to access files on various
storage sources in an universal fashion possibly on storage such as nfs
s/an universal/a universal/
with root squash we'll need to store the desired uid/gid in the
metadata.
Add new initialisation API that will store the desired uid/gid and a
wrapper for the current use. Additionally add docs for the two APIs.
---
src/storage/storage_backend.h | 3 +++
src/storage/storage_driver.c | 39 ++-
src/storage/storage_driver.h | 5 +++--
3 files changed, 44 insertions(+), 3 deletions(-)
+/**
+ * virStorageFileInitAs:
+ *
+ * @src: storage source definition
+ * @uid: uid to access the file as, -1 for current uid
+ * @gid: gid to access the file as, -1 for current gid
Correct grammar as written, but didn't flow well and took me two reads
to avoid confusion. Would be easier with the addition of or, as in:
@xid: xid to access the file as, or -1 for current xid
or even:
@xid: xid used to access the file, or -1 for current xid
+if (uid == (uid_t) -1)
+src-drv-uid = geteuid();
+else
+src-drv-uid = uid;
Do we need to do the conversion here, or can we store -1 and let other
routines later do the conversion? I'm not sure if it matters either
way, so I'm fine leaving it this way.
+int
+virStorageFileInit(virStorageSourcePtr src)
+{
+return virStorageFileInitAs(src, (uid_t) -1, (gid_t) -1);
Casts aren't strictly necessary on this line (the C compiler does the
correct conversion from int to uid_t thanks to the function prototype).
ACK with the comment and cast cleanup.
--
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
[libvirt] [PATCHv2] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
This introduces two new attributes cmd_per_lun and max_sectors same
with the names QEMU uses for virtio-scsi. An example of the XML:
controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50'
max_sectors='512'/
The corresponding QEMU command line:
-device virtio-scsi-pci,id=scsi0,cmd_per_lun=50,max_sectors=512,
bus=pci.0,addr=0x3
Signed-off-by: Mike Perez thin...@gmail.com
---
docs/formatdomain.html.in | 29 +++---
docs/schemas/domaincommon.rng | 10
src/conf/domain_conf.c | 27 ++--
src/conf/domain_conf.h | 2 ++
src/qemu/qemu_command.c| 27
.../qemuxml2argv-disk-virtio-scsi-cmd-per-lun.args | 9 +++
.../qemuxml2argv-disk-virtio-scsi-cmd-per-lun.xml | 29 ++
.../qemuxml2argv-disk-virtio-scsi-max-sectors.args | 9 +++
.../qemuxml2argv-disk-virtio-scsi-max-sectors.xml | 29 ++
tests/qemuxml2argvtest.c | 6 +
tests/qemuxml2xmltest.c| 2 ++
11 files changed, 168 insertions(+), 11 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-cmd-per-lun.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-cmd-per-lun.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-max-sectors.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-disk-virtio-scsi-max-sectors.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 691a451..8d44bc8 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2559,11 +2559,32 @@
p
An optional sub-element codedriver/code can specify the driver
- specific options. Currently it only supports attribute
codequeues/code
- (span class=since1.0.5/span, QEMU and KVM only), which specifies
the
- number of queues for the controller. For best performance, it's
recommended
- to specify a value matching the number of vCPUs.
+ specific options:
/p
+dl
+ dtcodequeues/code/dt
+ dd
+The optional codequeues/code attribute specifies the number of
+queues for the controller. For best performance, it's recommended to
+specify a value matching the number of vCPUs.
+span class=sinceSince 1.0.5 (QEMU and KVM only)/span
+ /dd
+ dtcodecmd-per-lun/code/dt
+ dd
+The optional codecmd-per-lun/code attribute specifies the maximum
+number of commands that can be queued on devices controlled by the
+host.
+span class=sinceSince 1.2.5 (QEMU and KVM only)/span
+ /dd
+ dtcodemax-sectors/code/dt
+ dd
+The optional codemax-sectors/code attribute specifies the maximum
+amount of data in bytes that will be transferred to or from the device
+in a single command. The transfer length is measured in sectors, where
+a sector is 512 bytes.
+span class=sinceSince 1.2.5 (QEMU and KVM only)/span
+ /dd
+/dl
p
USB companion controllers have an optional
sub-element codelt;mastergt;/code to specify the exact
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 4249ed5..d2ed8df 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1735,6 +1735,16 @@
ref name=unsignedInt/
/attribute
/optional
+optional
+ attribute name=cmd-per-lun
+ref name=unsignedInt/
+ /attribute
+/optional
+optional
+ attribute name=max-sectors
+ref name=unsignedInt/
+ /attribute
+/optional
/element
/optional
/interleave
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 40c385e..9132463 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6041,6 +6041,8 @@ virDomainControllerDefParseXML(xmlNodePtr node,
char *idx = NULL;
char *model = NULL;
char *queues = NULL;
+char *cmd_per_lun = NULL;
+char *max_sectors = NULL;
xmlNodePtr saved = ctxt-node;
int rc;
@@ -6084,6 +6086,8 @@ virDomainControllerDefParseXML(xmlNodePtr node,
if (cur-type == XML_ELEMENT_NODE) {
if (xmlStrEqual(cur-name, BAD_CAST driver))
queues = virXMLPropString(cur, queues);
+cmd_per_lun = virXMLPropString(cur, cmd-per-lun);
+max_sectors = virXMLPropString(cur, max-sectors);
}
cur = cur-next;
}
@@ -6094,6 +6098,17 @@ virDomainControllerDefParseXML(xmlNodePtr node,
goto error;
}
+if (cmd_per_lun virStrToLong_ui(cmd_per_lun, NULL, 10,
def-cmd_per_lun) 0) {
+virReportError(VIR_ERR_XML_ERROR,
+
Re: [libvirt] [PATCHv2 08/33] storage: Add storage file API to read file headers
On 05/22/2014 07:47 AM, Peter Krempa wrote:
Add storage driver based functions to access headers of storage files
for metadata extraction. Along with this patch a local filesystem and
gluster via libgfapi implementation is provided. The gluster
implementation is based on code of the saferead_lim function.
---
src/storage/storage_backend.h | 7
src/storage/storage_backend_fs.c | 30
src/storage/storage_backend_gluster.c | 66
+++
src/storage/storage_driver.c | 40 +
src/storage/storage_driver.h | 3 ++
5 files changed, 146 insertions(+)
+++ b/src/storage/storage_backend_gluster.c
@@ -638,6 +638,71 @@ virStorageFileBackendGlusterStat(virStorageSourcePtr src,
}
+static ssize_t
+virStorageFileBackendGlusterReadHeader(virStorageSourcePtr src,
+ ssize_t max_len,
+ char **buf)
+{
+virStorageFileBackendGlusterPrivPtr priv = src-drv-priv;
+glfs_fd_t *fd = NULL;
+size_t alloc = 0;
+size_t size = 0;
+int save_errno;
+ssize_t ret = -1;
+
+*buf = NULL;
+
+if (!(fd = glfs_open(priv-vol, src-path, O_RDONLY))) {
+virReportSystemError(errno, _(Failed to open file '%s'),
+ src-path);
+goto cleanup;
+}
+
+/* code below is shamelesly stolen from saferead_lim */
s/shamelesly/shamelessly/
+if (!src-drv-backend-storageFileReadHeader) {
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(storage file header reading is not supported for
+ storage type %s (protocol: %s)'),
+ virStorageTypeToString(src-type),
+ virStorageNetProtocolTypeToString(src-protocol));
Oh slick - the earlier patch that adds protocol==0 as none rather than
nbd makes this message tolerable even for non-network storage types.
Less special-casing is always good :)
+return -1;
Isn't this supposed to be 'return -2' according to the function
documentation?
ACK if you agree about the return value fix, and if it is the only
change you make besides the typo fix.
--
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] qemu: Add cmd_per_lun, max_sectors to virtio-scsi
On 16:06 Thu 22 May , Eric Blake wrote: On 05/22/2014 12:22 PM, Mike Perez wrote: This introduces two new attributes cmd_per_lun and max_sectors same with the names QEMU uses for virtio-scsi. An example of the XML: controller type='scsi' index='0' model='virtio-scsi' cmd_per_lun='50' max_sectors='512'/ I'm not a fan of underscore (why type the shift key, when a dash will do). The libvirt xml name does not have to exactly match the qemu option name, so maybe there's some room for bikeshedding what names we should actually present to the user. Thanks Eric. I posted v2 patch up which fixes the tab issues and switches the option names from underscores to dashes. -- Mike Perez -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 09/33] storage: backend: Add unique id retrieval API
On 05/22/2014 07:47 AM, Peter Krempa wrote:
Different protocols have different means to uniquely identify a storage
file. This patch implements a storage driver API to retrieve a unique
string describing a volume. The current implementation works for local
storage only and returns the canonical path of the volume.
To add caching support the local filesystem driver now has a private
structure holding the cached string, which is created only when it's
initially accessed.
This patch provides the implementation for local files only for start.
---
src/storage/storage_backend.h| 3 +++
src/storage/storage_backend_fs.c | 49
src/storage/storage_driver.c | 30
src/storage/storage_driver.h | 1 +
4 files changed, 83 insertions(+)
+typedef struct _virStorageFileBackendFsPriv virStorageFileBackendFsPriv;
+typedef virStorageFileBackendFsPriv *virStorageFileBackendFsPrivPtr;
+
+struct _virStorageFileBackendFsPriv {
+char *uid; /* unique file identifier (canonical path) */
The name 'uid' is misleading (too commonly used to mean user-id, which
it is not). Can you change it to something like 'fid' (short for
file-id) or 'canon'?
+};
+
+
static void
virStorageFileBackendFileDeinit(virStorageSourcePtr src)
{
@@ -1346,16 +1354,27 @@ virStorageFileBackendFileDeinit(virStorageSourcePtr
src)
virStorageTypeToString(virStorageSourceGetActualType(src)),
src-path);
+virStorageFileBackendFsPrivPtr priv = src-drv-priv;
+
+VIR_FREE(priv-uid);
As evidence, if I see this line in isolation, I think why are you
freeing an integer - I had to look at the header to learn oh, it's not
a usual uid, but a string.
+/*
+ * virStorageFileGetUniqueIdentifier: Get a unique string describing the
volume
+ *
+ * @src: file structure pointing to the file
+ *
+ * Returns a string uniquely describing a single volume (canonical path).
+ * The string shall not be freed. Returns NULL on error and sets a libvirt
+ * error code */
Is it also worth documenting that the string is invalidated when the
input parameter 'src' is freed?
The idea makes sense, but I'd like to see a v3.
--
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] qemu: Send migrate_cancel when aborting migration
-Original Message- From: libvir-list-boun...@redhat.com [mailto:libvir-list-boun...@redhat.com] On Behalf Of Jiri Denemark Sent: Thursday, May 22, 2014 7:55 PM To: libvir-list@redhat.com Subject: [libvirt] [PATCH 2/3] qemu: Send migrate_cancel when aborting migration When QEMU reports failed or cancelled migration, we don't need to send it migrate_cancel QMP command. But in all other error paths, such as if we detect broken connection to a destination daemon or something else happens inside libvirt, we need to explicitly send migrate_cancel command instead of relying on the migration to be implicitly cancelled when destination QEMU is killed. Because we were not doing so, one could end up with a paused domain after failed migration. https://bugzilla.redhat.com/show_bug.cgi?id=807023 Hi, Jiri I'm interested in the patch. I want to know the specific scene. But the bug link you posted is about device_del. Is there another link about migration? (I think) Signed-off-by: Jiri Denemark jdene...@redhat.com --- -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: Fix specifying char devs for PPC
[reposting to the list with permission] On 05/22/2014 06:37 PM, hong-hua@freescale.com wrote: Hi Eric, Thanks for your comments. I also prefer the second reply threading. Exactly I use git send-email to post patches to this mailing list. $ git send-email -cover-letter.patch 0001-change-machine-name-ppce500v2-as-ppce500.patch 0002-qemu-Fix-specifying-char-devs-for-PPC.patch 0003-tests-add-test-case-for-serial-option-for-ppce500.patch --to=libvir-list@redhat.com $ git config --global -l sendemail.smtpserver=remotesmtp.freescale.net sendemail.chainreplyto=true This is the culprit. Per 'git send-email --help', the git default of sendemail.chainreplyto=false gives the style we prefer. sendemail.confirm=auto Is there any configuration for git to follow up? Depends on whether you want to change the settings for all repositories on your computer (~/.gitconfig, or 'git config --global') or just for your copy of libvirt.git (libvirt/.git/config, or plain 'git config'). We also tend to avoid top-posting on the list. -- 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] [PATCHv2 3/4] qemu: fix RTC_CHANGE event for clock offset='variable' basis='utc'/
On Thu, May 22, 2014 at 01:33:14PM -0600, Eric Blake wrote: [Adding qemu] On 05/22/2014 05:07 AM, Laine Stump wrote: commit e31b5cf393857 attempted to fix libvirt's VIR_DOMAIN_EVENT_ID_RTC_CHANGE, which is documentated to always s/documentated/documented/ provide the new offset of the domain's real time clock from UTC. The problem was that, in the case that qemu is provided with an -rtc base=x where x is an absolute time (rather than utc or localtime), the offset sent by qemu's RTC_CHANGE event is *not* the new offset from UTC, but rather is the sum of all changes to the domain's RTC since it was started with base=x. So, despite what was said in commit e31b5cf393857, if we assume that the original value stored in adjustment was the offset from UTC at the time the domain was started, we can always determine the current offset from UTC by simply adding the most recent (i.e. current) offset from qemu to that original adjustment. Is this true even if we miss an RTC update event from qemu? I'm worried about the following situation: user prepares to do a libvirtd upgrade, so libvirtd is shut down. If adjustment0 field is updated from adjustment, via a libvirtd shutdown, the current patch will also break, i believe. Not sure if thats possible, though. Then the guest triggers an RTC update, so qemu sends an event, but the event is lost. Then libvirtd starts again, and doesn't realize the event is lost. Yes, but that case is also true for any other QMP asynchronous event, and therefore should be handled generically i suppose (QMP channel data should be maintained across libvirtd shutdown). Luiz? Do we need more help from qemu, such as a new field to an existing QMP command (or a new QMP command) that lists the cumulative offset that qemu is using, where we call that query command any time after an RTC update event or after a libvirtd restart? I'm wondering if this is more a bug in qemu for not providing the right information rather than libvirt's responsibility to work around it. If the only way to keep accurate information is to sum the values we get from events, we are at risk of a lost event getting us messed up. Good point, unsure whether its specific to this command, though. Could add a new QMP command to query the RTC offset, yes. This patch accomplishes that by storing the initial adjustment in the domain's status as adjustment0. Each time a new RTC_CHANGE event is received from qemu, we simply add adjustment0 to the value sent by qemu, store that as the new adjustment, and forward that value on to any event handler. This patch (*not* e31b5cf393857, which should be reverted prior to applying this patch) fixes: https://bugzilla.redhat.com/show_bug.cgi?id=964177 (for the case where basis='utc'. It does not fix basis='localtime') --- Changes from V1: remove all attempts to fix basis='localtime' in favor of fixing it in a simpler and better manner in a separate patch. I'd also appreciate it if the qemu developers can chime in on what is supposed to happen for localtime guests. There are at least four combinations: host running on UTC bios time, guest running on UTC time (in my opinion, the only sane setting, but we're talking about reality not sanity) 1) Why does Windows keep your BIOS clock on local time? http://blogs.msdn.com/b/oldnewthing/archive/2004/09/02/224672.aspx 2) Windows with RTC UTC https://wiki.archlinux.org/index.php/Time#UTC_in_Windows host running on UTC, guest running on localtime (perhaps the guest is windows, and we know that windows prefers to run on localtime) Its the default, thats all. See 1) above. host running on localtime bios (perhaps because it is dual-boot with windows, and windows prefers bios in localtime), guest running on UTC time Can't see why hosts using localtime or UTC is relevant. Assume host is synchronized to UTC via NTP (so you can use UTC or convert to localtime if desired). host running on localtime, guest running on localtime But it gets even more complicated. The host localtime need not be consistent with the guest localtime. That is, I could be a cloud provider with servers on the east coast, and renting out processor time to a client on the west coast that wants their guest tied to west coast localtime. And that's assuming that both host and guest switch in and out of daylight savings at the same time, which falls apart when you cross political boundaries. Then there's the fun of migration (what if my server farm is spread across multiple timezones - does migration take into account the difference in localtime between source and destination servers). I can _totally_ understand the desire to run a GUEST in such a way that the guest thinks it has a bios stored in localtime (and when the guest updates the RTC twice a year to account for daylight savings, it changes what offset we track about the guest). But I think it is
[libvirt] GSoC student introduction and work plan
Hi, everyone, My name is Taowei Luo. I'm one of the students working for
GSoC this summer. My project is rewriting the vbox driver.
Before coding, I really need some discussions about how to implement it.
Here I post my first version of plan. I had show as much details as I
thought about in this plan. If anyone have ideas or suggestions about my
work, I would like to hear. My nick name in #virt IRC channel is uaedante
(or dante I used before). If I am offline in IRC, please contract me with
my email address uaeda...@gmail.com.
Regards,
Taowei.
Plan to rewrite vbox driver
Introduction:
The existing vbox driver codes use vbox API directly. But the vbox API is
incompatible between different versions. To support all vbox API versions,
the vbox_tmpl.c file provides a template to implement all vbox drivers.
Each specified version of vbox driver includes the vbox_tmpl.c file, with
the definition in the corresponding head file, they work well. As the vbox
API defined in head files is conflict between different versions, even some
behavior of vbox APIs are changed. The vbox_tmpl.c contains lots of codes
such as “#if VBOX_API_VERSION 4001000”. These codes is hard to manage or
adding new features.
Plan:
The core idea of my rewriting work is to add a middle layer to uniform the
API changes. This modification will not change any functions’ behavior or
sematic.
The middle layer has a global structure which contains API changes. Like
this:
struct vboxUniformedAPI {
A (*funcA)(A1 a, ….);
B (*funcB)(B1 a, ….);
}
If there is a code segment:
#if VBOX_API_VERSION 4001000
adapter-vtbl-GetHostInterface(adapter, hostIntUtf16);
#else /* VBOX_API_VERSION = 4001000 */
adapter-vtbl-GetBridgedInterface(adapter, hostIntUtf16);
#endif /* VBOX_API_VERSION = 4001000 */
This will result to a variable in vboxUniformedAPI structure:
RetType (*GetInterface)(INetworkAdapter *a, PRUnichar *b)
When using the vbox under 4.1, this variable will points to a function that
calls GetHostInterface. When using the vbox later than 4.1 and the variable
will points to a function which calls GetBridgedInterface.
When replace these code segments with a single function, we need to do
living variable analyze and find out what is used and what is changed in
these codes. I plan to do these analyze manually.
All conflict in function prototype or function implementation will be fixed
up though this method.
When meeting a conflict in variable type, like:
#if VBOX_API_VERSION 4001000
TypeA var;
#else /* VBOX_API_VERSION = 4001000 */
TypeB var;
#endif /* VBOX_API_VERSION = 4001000 */
I will add a union like this:
Union TypeC{
TypeA a;// used in some versions
TypeB b;// used in some other versions
}
If there is any operation using variables with such a union type (or
functions use it as an argument or return value), I will add a new function
in vboxUniformedAPI to handle this operation. These functions use TypeC as
parameter type, dispatch it depends on the vbox version and return TypeC
value to conceal type conflict.
We still have another kind of conflict. There are some versions have more
variables, operations or function calls than other versions. I will reserve
all variables even though they may not be used in some situation. And put
an empty function if they have nothing to do in the specified function
step. (I have a backup solution here. That is adding some flags to indicate
whether the procession is necessary in the API version and prevent upper
layer from calling a nonexistent function.)
Expected result:
After rewriting with the rules above, we will have a new vbox_tmpl.c. It
gathers version specified code and common code respectively. The common
codes use stable vbox API or functions in vboxUniformedAPI. A function
named InstallvboxUnifromedAPI will fill the vboxUniformedAPI structure
depends on the current vbox API version. In final step, I will put all vbox
APIs into the vboxUniformedAPI and make the vbox driver codes use my API
only. If this is done, the vbox driver will not be complied for each
version anymore.
Implementation:
First, define the vboxUniformedAPI structure in vbox_tmpl.c, and write the
InstallvboxUnifromedAPI function for every vbox API version.
Next, rewrite the functions which have conflict codes one by one. Each time
rewrite a new function, we can do some tests (here, I am not sure what kind
of tests is enough). When rewriting one function in vbox_tmpl.c, the others
will not change.
At this point, there will split version specified codes and common codes
But we still need to build the whole vbox_tmpl.c for each vbox API version.
If we want a single object file be generated by vbox_tmpl.c, we need to
prevent vbox_tmpl.c from calling vbox API directly. This could be done by
putting all of the vbox API calls into the middle vboxUniformedAPI layer,
and moving common codes into vbox_common.c. The vbox_common.c should only
use APIs in vboxUniformedAPI and will be
