Re: [libvirt] [PATCH v4 1/3] conf: Extend loader/ and introduce nvram/
On 08/21/14 10:50, Michal Privoznik wrote: Up to now, users can configure BIOS via the loader/ element. With the upcoming implementation of UEFI this is not enough as BIOS and UEFI are conceptually different. For instance, while BIOS is ROM, UEFI is programmable flash (although all writes to code section are denied). Therefore we need new attribute @type which will differentiate the two. Then, new attribute @readonly is introduced to reflect the fact that some images are RO. Moreover, the OVMF (which is going to be used mostly), works in two modes: 1) Code and UEFI variable store is mixed in one file. 2) Code and UEFI variable store is separated in two files The latter has advantage of updating the UEFI code without losing the configuration. However, in order to represent the latter case we need yet another XML element: nvram/. Currently, it has no additional attributes, it's just a bare element containing path to the variable store file. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- docs/formatdomain.html.in | 19 - docs/schemas/domaincommon.rng | 21 ++ src/conf/domain_conf.c | 87 +- src/conf/domain_conf.h | 22 +- src/libvirt_private.syms | 3 + src/qemu/qemu_command.c| 5 +- src/security/virt-aa-helper.c | 4 +- src/vbox/vbox_common.c | 7 +- src/xenapi/xenapi_driver.c | 3 +- src/xenconfig/xen_common.c | 7 +- src/xenconfig/xen_sxpr.c | 16 ++-- tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml | 40 ++ .../qemuxml2xmlout-pci-bridge-many-disks.xml | 2 +- tests/qemuxml2xmltest.c| 2 + tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml | 2 +- .../sexpr2xml-fv-serial-dev-2-ports.xml| 2 +- .../sexpr2xml-fv-serial-dev-2nd-port.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml | 2 +- .../sexpr2xml-fv-serial-tcp-telnet.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-sound.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-v2.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml | 2 +- tests/xmconfigdata/test-escape-paths.xml | 2 +- tests/xmconfigdata/test-fullvirt-force-hpet.xml| 2 +- tests/xmconfigdata/test-fullvirt-force-nohpet.xml | 2 +- tests/xmconfigdata/test-fullvirt-localtime.xml | 2 +- tests/xmconfigdata/test-fullvirt-net-ioemu.xml | 2 +- tests/xmconfigdata/test-fullvirt-net-netfront.xml | 2 +- tests/xmconfigdata/test-fullvirt-new-cdrom.xml | 2 +- tests/xmconfigdata/test-fullvirt-old-cdrom.xml | 2 +- tests/xmconfigdata/test-fullvirt-parallel-tcp.xml | 2 +- .../test-fullvirt-serial-dev-2-ports.xml | 2 +- .../test-fullvirt-serial-dev-2nd-port.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-file.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-null.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-pipe.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-pty.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-stdio.xml | 2 +- .../test-fullvirt-serial-tcp-telnet.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-tcp.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-udp.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-unix.xml | 2 +- tests/xmconfigdata/test-fullvirt-sound.xml | 2 +-
Re: [libvirt] [PATCH v4 2/3] qemu: Implement extended loader and nvram
On 08/21/14 10:50, Michal Privoznik wrote: QEMU now supports UEFI with the following command line: -drive file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \ -drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \ where the first line reflects loader and the second one nvram. Moreover, these two lines obsoletes the -bios argument. Note that UEFI is unusable without ACPI. This is handled properly now. Among with this extension, the variable file is expected to be writable and hence we need security drivers to label it. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- src/qemu/qemu_command.c| 94 +- src/security/security_dac.c| 8 ++ src/security/security_selinux.c| 8 ++ .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args | 10 +++ tests/qemuxml2argvtest.c | 2 + 5 files changed, 118 insertions(+), 4 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args Acked-by: Laszlo Ersek ler...@redhat.com (Please append this line to the commit message in any further reposts.) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 3/3] qemu: Automatically create NVRAM store
comments below
On 08/21/14 10:50, Michal Privoznik wrote:
When using split UEFI image, it may come handy if libvirt manages per
domain _VARS file automatically. While the _CODE file is RO and can be
shared among multiple domains, you certainly don't want to do that on
the _VARS file. This latter one needs to be per domain. So at the
domain startup process, if it's determined that domain needs _VARS
file it's copied from this master _VARS file. The location of the
master file is configurable in qemu.conf.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
libvirt.spec.in| 2 +
src/Makefile.am| 1 +
src/qemu/libvirtd_qemu.aug | 3 +
src/qemu/qemu.conf | 14
src/qemu/qemu_conf.c | 93 ++
src/qemu/qemu_conf.h | 5 ++
src/qemu/qemu_process.c| 132
+
src/qemu/test_libvirtd_qemu.aug.in | 3 +
8 files changed, 253 insertions(+)
I compared this patch with its v2 counterpart. I can see that all of my
remarks have been addressed. Two notes:
@@ -654,6 +711,42 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr
cfg,
GET_VALUE_BOOL(log_timestamp, cfg-logTimestamp);
+CHECK_TYPE(nvram, VIR_CONF_LIST);
+if ((p = virConfGetValue(conf, nvram))) {
+size_t len;
+virConfValuePtr pp;
+
+while (cfg-nloader) {
+VIR_FREE(cfg-loader[cfg-nloader - 1]);
+VIR_FREE(cfg-nvram[cfg-nloader - 1]);
+cfg-nloader--;
+}
+VIR_FREE(cfg-loader);
+VIR_FREE(cfg-nvram);
+
+/* Calc length and check items */
+for (len = 0, pp = p-list; pp; len++, pp = pp-next) {
+if (pp-type != VIR_CONF_STRING) {
+virReportError(VIR_ERR_CONF_SYNTAX, %s,
+ _(nvram must be a list of strings));
+goto cleanup;
+}
+}
+
+if (len
+(VIR_ALLOC_N(cfg-loader, len) 0 ||
+ VIR_ALLOC_N(cfg-nvram, len) 0))
+goto cleanup;
+cfg-nloader = len;
+
+for (i = 0, pp = p-list; pp; i++, pp = pp-next) {
+if (virQEMUDriverConfigNVRAMParse(pp-str,
+ cfg-loader[i],
+ cfg-nvram[i]) 0)
+goto cleanup;
+}
+}
+
ret = 0;
cleanup:
(a) In general, this looks like a good solution. This prevents freeing
garbage pointers, and also handles the nvram=[] (empty list) case:
nvram=[] will work as if nvram were absent completely. Okay.
(b) However, I think CHECK_TYPE() is used incorrectly:
'p = virConfGetValue(conf, nvram)' must be done *before* CHECK_TYPE().
You didn't catch this in testing because the value of p, before you
reach CHECK_TYPE(), has been set by
p = virConfGetValue(conf, hugetlbfs_mount);
That is, p was most likely NULL in your testing. And p == NULL
short-circuits CHECK_TYPE(), to success.
You need:
p = virConfGetValue(conf, nvram);
CHECK_TYPE(nvram, VIR_CONF_LIST);
if (p) {
Refer to the cgroup_controllers block in
virQEMUDriverConfigLoadFile(), a little bit higher up; that one uses
this same pattern.
The rest seems fine to me.
Thanks!
Laszlo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 3/3] qemu: Automatically create NVRAM store
On 08/21/14 11:05, Daniel P. Berrange wrote:
On Thu, Aug 21, 2014 at 10:50:24AM +0200, Michal Privoznik wrote:
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 7bbbe09..79bba36 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -487,3 +487,17 @@
# Defaults to 1.
#
#log_timestamp = 0
+
+
+# Location of master nvram file
+#
+# When a domain is configured to use UEFI instead of standard
+# BIOS it may use a separate storage for UEFI variables. If
+# that's the case libvirt creates the variable store per domain
+# using this master file as image. Each UEFI firmware can,
+# however, have different variables store. Therefore the nvram is
+# a list of strings when a single item is in form of:
+# ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}.
+# Later, when libvirt creates per domain variable store, this
+# list is searched for the master image.
+#nvram = [ /usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd ]
So the user has the ability to specify a arbitrary BIOS in the XML,
but unless it matches one of the ones listed in the libvirt config
they aren't going to be able to start the guest. What can we do
about this, as it doesn't really seem like a great position to be
in.
I disagree. Users who use virt-manager (for which patches still have to
be written, to expose this feature) won't put arbitrary strings in the
loader element; virt-manager should offer a minimal choice between
BIOS vs. UEFI.
Users who are hard-core enough to hack the domain XML by hand are
expected to provide good values.
In addition, users are already able to specify an arbitrary firmware
file in the loader element, and if the value doesn't match a valid
firmware binary file on the host, then the guest won't start. Same
responsibility for the user.
I wonder if we should have explicitly record the template in the
XML too. eg
loader.../loader
nvram template=../nvram
If no template attribute is given, then don't try to automatically
create the nvram file at all.
I don't see how this would help with anything. If the template is not
provided, then you immediately branch to the failure case that you don't
seem to like above (ie. no varstore file exists for the guest).
And, if the user *wants* to provde a varstore template, then the source
of *that* bit of information is precisely the same as the one of the
loader and nvram elements: the user himself. If the user can be
trusted (expected) to provide a valid varstore template in
nvram/@template, then he can be trusted (expected) just the same to pick
a firmware binary that is listed in the config file.
If the user wants a completely custom firmware, then he can provide both
loader and nvram up-front, in which case the config file won't be
consulted.
The varstore template is not a VM-level property. The varstore template
is a property of the firmware binary.
Just require the user to pre-create
it. That lets us avoid the global config parameters and any reliance
on OVMF file naming conventions shown below
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index baa866a..6f79a17 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -67,6 +67,7 @@
#include virstring.h
#include virhostdev.h
#include storage/storage_driver.h
+#include configmake.h
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -3734,6 +3735,130 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
}
+static int
+qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
+ virDomainDefPtr def,
+ bool migrated)
+{
+int ret = -1;
+int srcFD = -1;
+int dstFD = -1;
+virDomainLoaderDefPtr loader = def-os.loader;
+bool generated = false;
+bool created = false;
+
+/* Unless domain has RO loader of pflash type, we have
+ * nothing to do here. If the loader is RW then it's not
+ * using split code and vars feature, so no nvram file needs
+ * to be created. */
+if (!loader || loader-type != VIR_DOMAIN_LOADER_TYPE_PFLASH ||
+loader-readonly != VIR_TRISTATE_SWITCH_ON)
+return 0;
+
+/* If the nvram path is configured already, there's nothing
+ * we need to do. Unless we are starting the destination side
+ * of migration in which case nvram is configured in the
+ * domain XML but the file doesn't exist yet. Moreover, after
+ * the migration is completed, qemu will invoke a
+ * synchronization write into the nvram file so we don't have
+ * to take care about transmitting the real data on the other
+ * side. */
+if (loader-nvram !migrated)
+return 0;
+
+/* Autogenerate nvram path if needed.*/
+if (!loader-nvram) {
+if (virAsprintf(loader-nvram,
+%s/lib/libvirt/qemu/nvram/%s_VARS.fd,
+LOCALSTATEDIR, def-name) 0)
+goto cleanup;
This seems rather x86 OVMF specific in naming. It isn't going
Re: [libvirt] RFC: sVirt disk isolation with network based storage
On Thu, Aug 21, 2014 at 03:47:14PM -0400, Paul Moore wrote: On Thursday, August 21, 2014 10:48:05 AM Daniel J Walsh wrote: I think we should setup a meeting to discuss this and figure out our option. Sorry I'm slow to the party, I'm at LSS/LinuxCon this week and the network has been fairly spotty. We need a mechanism for libvirt to send the labels of the process and images to the remote server and then we need an enforcement mechanism to only allow the process label to interact with the file image. SELinux could do this if each vm has a separate process running on the server interacting with the image. Otherwise the server needs to do some kind of enforcement on its own. We could use some form of labeled networking for transmitting the MCS Label of qemu to the server or we would need to extend the protocol to send the label down. There is two ways to handle labeled networking.The most common labeling standard,CIPSO, only sends the MCS portion of the label. The second form can send the entire label of the process, but it is seldom used and requires Labeled IPSEC. As one would expect, neither CIPSO or labeled IPsec are prefect, but they are really our only options for conveying labels across a network - unless we want to augment/extend RBD, which I know almost nothing about (a quick search makes me think this is Ceph's remote storage protocol). I'm afraid I don't think that passing labels at the RBD protocol level is going to fly because the RBD client here is the QEMU process and we can not trust the QEMU process to be honest in the data it sends to the RBD server. We can only trust the kernel / libvirt which I think means that CIPSO / IPsec are the only trustworthy options here. Daniel (Mr. Libvirt, not Mr. SELinux), can you provide a quick overview of RBD, with bonus points for information on who controls the protocol (Inktank/RH or IETF) and if it offers any sort of extensibility (in other words, is there any hope for us to add label information to the protocol). Speaking mostly from a position of ignorance on the matter, I assume it is controlled by Inktank developers, as I've not heard any mention of a standardization effort. But I don't think that matters for the reason above. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 3/3] qemu: Automatically create NVRAM store
On Fri, Aug 22, 2014 at 10:27:29AM +0200, Laszlo Ersek wrote:
On 08/21/14 11:05, Daniel P. Berrange wrote:
On Thu, Aug 21, 2014 at 10:50:24AM +0200, Michal Privoznik wrote:
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 7bbbe09..79bba36 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -487,3 +487,17 @@
# Defaults to 1.
#
#log_timestamp = 0
+
+
+# Location of master nvram file
+#
+# When a domain is configured to use UEFI instead of standard
+# BIOS it may use a separate storage for UEFI variables. If
+# that's the case libvirt creates the variable store per domain
+# using this master file as image. Each UEFI firmware can,
+# however, have different variables store. Therefore the nvram is
+# a list of strings when a single item is in form of:
+# ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}.
+# Later, when libvirt creates per domain variable store, this
+# list is searched for the master image.
+#nvram = [ /usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd ]
So the user has the ability to specify a arbitrary BIOS in the XML,
but unless it matches one of the ones listed in the libvirt config
they aren't going to be able to start the guest. What can we do
about this, as it doesn't really seem like a great position to be
in.
I disagree. Users who use virt-manager (for which patches still have to
be written, to expose this feature) won't put arbitrary strings in the
loader element; virt-manager should offer a minimal choice between
BIOS vs. UEFI.
Users who are hard-core enough to hack the domain XML by hand are
expected to provide good values.
The problem I'm raising is that it is *not* sufficient to merely
provide good values in the XML here. You can't simply deploy a
custom OVMF file and update your XML, because this code is relying
on values in the libvirtd.conf configuration file.
I wonder if we should have explicitly record the template in the
XML too. eg
loader.../loader
nvram template=../nvram
If no template attribute is given, then don't try to automatically
create the nvram file at all.
I don't see how this would help with anything. If the template is not
provided, then you immediately branch to the failure case that you don't
seem to like above (ie. no varstore file exists for the guest).
It helps because it allows you to setup custom OVMF builds without
having to edit libvirtd.conf - the information that would go into
the libvirtd.conf file is instead provided directly in the XML.
And, if the user *wants* to provde a varstore template, then the source
of *that* bit of information is precisely the same as the one of the
loader and nvram elements: the user himself. If the user can be
trusted (expected) to provide a valid varstore template in
nvram/@template, then he can be trusted (expected) just the same to pick
a firmware binary that is listed in the config file.
Nope, that's giving two different experiances. If you use the default
OVMF file, then you don't need to care about manually setting up the
NVRAM store for each guest becaue libvirt can copy the template. If
you use a custom OVMF file apps have to manually setup NVRAM store
per guest. This sucks. The solution of including the nvram template
in the XML allows us to have a consistent experiance where libvirt
is always able to automatically create the per-guest NVRAM file.
The varstore template is not a VM-level property. The varstore template
is a property of the firmware binary.
Yes, and we're providing the firmware in the XML so it is reasonable
to provide the varstore template location in the XML too, instead of
having to re-configure libvirtd.conf and restart libvirtd to use it.
Just require the user to pre-create
it. That lets us avoid the global config parameters and any reliance
on OVMF file naming conventions shown below
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index baa866a..6f79a17 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -67,6 +67,7 @@
#include virstring.h
#include virhostdev.h
#include storage/storage_driver.h
+#include configmake.h
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -3734,6 +3735,130 @@ qemuProcessVerifyGuestCPU(virQEMUDriverPtr driver,
}
+static int
+qemuPrepareNVRAM(virQEMUDriverConfigPtr cfg,
+ virDomainDefPtr def,
+ bool migrated)
+{
+int ret = -1;
+int srcFD = -1;
+int dstFD = -1;
+virDomainLoaderDefPtr loader = def-os.loader;
+bool generated = false;
+bool created = false;
+
+/* Unless domain has RO loader of pflash type, we have
+ * nothing to do here. If the loader is RW then it's not
+ * using split code and vars feature, so no nvram file needs
+ * to be created. */
+if (!loader || loader-type != VIR_DOMAIN_LOADER_TYPE_PFLASH ||
+loader-readonly !=
[libvirt] [PATCH] build: fix mingw build with virCommandReorderFDs
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
Notes:
pushed as a build-breaker
src/util/vircommand.c | 4 ++--
src/util/virutil.c| 12
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 3b3e6f5..1d6dbd9 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -201,6 +201,8 @@ virCommandFDSet(virCommandPtr cmd,
return 0;
}
+#ifndef WIN32
+
static void
virCommandReorderFDs(virCommandPtr cmd)
{
@@ -273,8 +275,6 @@ virCommandReorderFDs(virCommandPtr cmd)
return;
}
-#ifndef WIN32
-
/**
* virFork:
*
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 1d897d9..2edbec5 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -2397,6 +2397,8 @@ void virUpdateSelfLastChanged(const char *path)
}
}
+#ifndef WIN32
+
/**
* virGetListenFDs:
*
@@ -2458,3 +2460,13 @@ virGetListenFDs(void)
return nfds;
}
+
+#else /* WIN32 */
+
+unsigned int
+virGetListenFDs(void)
+{
+return 0;
+}
+
+#endif /* WIN32 */
--
2.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] vbox: Register per partes
Since times when vbox moved to the daemon (due to some licensing issue) the subdrivers that vbox implements were registered, but not opened since our generic subdrivers took priority. I've tried to fix this in 65b7d553f39ff9 but it was not correct. Apparently moving vbox driver registration upfront changes the default connection URI which makes some users sad. So, this commit breaks vbox into pieces and register vbox's network and storage drivers first, and vbox driver then at the end. This way, the vbox driver is registered in the order it always was, but its subdrivers are registered prior the generic ones. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- daemon/libvirtd.c | 16 ++-- src/Makefile.am| 41 ++--- src/vbox/vbox_driver.c | 50 +- src/vbox/vbox_driver.h | 10 ++ 4 files changed, 107 insertions(+), 10 deletions(-) diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c index 4cf78e6..c3bd2ab 100644 --- a/daemon/libvirtd.c +++ b/daemon/libvirtd.c @@ -383,7 +383,7 @@ static void daemonInitialize(void) * is not loaded they'll get a suitable error at that point */ # ifdef WITH_VBOX -virDriverLoadModule(vbox); +virDriverLoadModule(vbox_network); # endif # ifdef WITH_NETWORK virDriverLoadModule(network); @@ -391,6 +391,9 @@ static void daemonInitialize(void) # ifdef WITH_INTERFACE virDriverLoadModule(interface); # endif +# ifdef WITH_VBOX +virDriverLoadModule(vbox_storage); +# endif # ifdef WITH_STORAGE virDriverLoadModule(storage); # endif @@ -418,12 +421,15 @@ static void daemonInitialize(void) # ifdef WITH_UML virDriverLoadModule(uml); # endif +# ifdef WITH_VBOX +virDriverLoadModule(vbox); +# endif # ifdef WITH_BHYVE virDriverLoadModule(bhyve); # endif #else # ifdef WITH_VBOX -vboxRegister(); +vboxNetworkRegister(); # endif # ifdef WITH_NETWORK networkRegister(); @@ -431,6 +437,9 @@ static void daemonInitialize(void) # ifdef WITH_INTERFACE interfaceRegister(); # endif +# ifdef WITH_VBOX +vboxStorageRegister(); +# endif # ifdef WITH_STORAGE storageRegister(); # endif @@ -458,6 +467,9 @@ static void daemonInitialize(void) # ifdef WITH_UML umlRegister(); # endif +# ifdef WITH_VBOX +vboxRegister(); +# endif # ifdef WITH_BHYVE bhyveRegister(); # endif diff --git a/src/Makefile.am b/src/Makefile.am index 538530e..46e411e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1135,13 +1135,27 @@ libvirt_driver_vmware_la_SOURCES = $(VMWARE_DRIVER_SOURCES) endif WITH_VMWARE if WITH_VBOX -noinst_LTLIBRARIES += libvirt_driver_vbox_impl.la +noinst_LTLIBRARIES += \ + libvirt_driver_vbox_impl.la \ + libvirt_driver_vbox_network_impl.la \ + libvirt_driver_vbox_storage_impl.la libvirt_driver_vbox_la_SOURCES = libvirt_driver_vbox_la_LIBADD = libvirt_driver_vbox_impl.la +libvirt_driver_vbox_network_la_SOURCES = +libvirt_driver_vbox_network_la_LIBADD = libvirt_driver_vbox_network_impl.la +libvirt_driver_vbox_storage_la_SOURCES = +libvirt_driver_vbox_storage_la_LIBADD = libvirt_driver_vbox_storage_impl.la if WITH_DRIVER_MODULES -mod_LTLIBRARIES += libvirt_driver_vbox.la +mod_LTLIBRARIES += \ + libvirt_driver_vbox.la \ + libvirt_driver_vbox_network.la \ + libvirt_driver_vbox_storage.la libvirt_driver_vbox_la_LIBADD += ../gnulib/lib/libgnu.la libvirt_driver_vbox_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_vbox_network_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_vbox_network_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_vbox_storage_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_vbox_storage_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) else ! WITH_DRIVER_MODULES noinst_LTLIBRARIES += libvirt_driver_vbox.la # GPLv2-only license requries that it be linked into @@ -1151,12 +1165,33 @@ endif ! WITH_DRIVER_MODULES libvirt_driver_vbox_impl_la_CFLAGS = \ -I$(top_srcdir)/src/conf\ - $(AM_CFLAGS) + $(AM_CFLAGS)\ + -DVBOX_DRIVER libvirt_driver_vbox_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_vbox_impl_la_LIBADD = $(DLOPEN_LIBS) \ $(MSCOM_LIBS) \ $(LIBXML_LIBS) libvirt_driver_vbox_impl_la_SOURCES = $(VBOX_DRIVER_SOURCES) + +libvirt_driver_vbox_network_impl_la_CFLAGS = \ + -I$(top_srcdir)/src/conf\ + $(AM_CFLAGS)\ + -DVBOX_NETWORK_DRIVER +libvirt_driver_vbox_network_impl_la_LDFLAGS = $(AM_LDFLAGS) +libvirt_driver_vbox_network_impl_la_LIBADD = $(DLOPEN_LIBS)\ + $(MSCOM_LIBS) \ + $(LIBXML_LIBS)
[libvirt] [PATCH 1/3] virdrivermoduletest: Test all the modules
Even though we kept adding new and new modules (e.g. vbox or bhyve) the test wasn't updated. Do that now. Moreover, while it's not crucial, it's nice to reorder test cases to match the order in which the daemon loads the modules. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- tests/virdrivermoduletest.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/tests/virdrivermoduletest.c b/tests/virdrivermoduletest.c index 840fc28..d823ad9 100644 --- a/tests/virdrivermoduletest.c +++ b/tests/virdrivermoduletest.c @@ -71,6 +71,9 @@ mymain(void) #else # define USE_NETWORK NULL #endif +#ifdef WITH_INTERFACE +TEST(interface, NULL); +#endif #ifdef WITH_STORAGE TEST(storage, NULL); #endif @@ -83,8 +86,11 @@ mymain(void) #ifdef WITH_NWFILTER TEST(nwfilter, NULL); #endif -#ifdef WITH_INTERFACE -TEST(interface, NULL); +#ifdef WITH_XEN +TEST(xen, NULL); +#endif +#ifdef WITH_LIBXL +TEST(libxl, NULL); #endif #ifdef WITH_QEMU TEST(qemu, USE_NETWORK); @@ -95,11 +101,11 @@ mymain(void) #ifdef WITH_UML TEST(uml, NULL); #endif -#ifdef WITH_XEN -TEST(xen, NULL); +#ifdef WITH_VBOX +TEST(vbox, NULL); #endif -#ifdef WITH_LIBXL -TEST(libxl, NULL); +#ifdef WITH_BHYVE +TEST(bhyve, NULL); #endif return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Fix VirtualBox registration
This is supposed to fix bug report which appeared on the list [1]. The driver registration ordering is back as it was, but the subdrivers are registered before. Unfortunately, if we don't want to dig vbox network and storage code out, we have to compile the sources couple of times. Michal Privoznik (3): virdrivermoduletest: Test all the modules virDriverLoadModule: Honor libvirt func name tranlsation vbox: Register per partes daemon/libvirtd.c | 16 +-- src/Makefile.am | 41 ++--- src/driver.c| 19 +++-- src/vbox/vbox_driver.c | 50 - src/vbox/vbox_driver.h | 10 + tests/virdrivermoduletest.c | 18 ++-- 6 files changed, 136 insertions(+), 18 deletions(-) -- 1.8.5.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 3/3] qemu: Automatically create NVRAM store
On 08/22/14 10:54, Daniel P. Berrange wrote:
On Fri, Aug 22, 2014 at 10:27:29AM +0200, Laszlo Ersek wrote:
On 08/21/14 11:05, Daniel P. Berrange wrote:
On Thu, Aug 21, 2014 at 10:50:24AM +0200, Michal Privoznik wrote:
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 7bbbe09..79bba36 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -487,3 +487,17 @@
# Defaults to 1.
#
#log_timestamp = 0
+
+
+# Location of master nvram file
+#
+# When a domain is configured to use UEFI instead of standard
+# BIOS it may use a separate storage for UEFI variables. If
+# that's the case libvirt creates the variable store per domain
+# using this master file as image. Each UEFI firmware can,
+# however, have different variables store. Therefore the nvram is
+# a list of strings when a single item is in form of:
+# ${PATH_TO_UEFI_FW}:${PATH_TO_UEFI_VARS}.
+# Later, when libvirt creates per domain variable store, this
+# list is searched for the master image.
+#nvram = [ /usr/share/OVMF/OVMF_CODE.fd:/usr/share/OVMF/OVMF_VARS.fd ]
So the user has the ability to specify a arbitrary BIOS in the XML,
but unless it matches one of the ones listed in the libvirt config
they aren't going to be able to start the guest. What can we do
about this, as it doesn't really seem like a great position to be
in.
I disagree. Users who use virt-manager (for which patches still have to
be written, to expose this feature) won't put arbitrary strings in the
loader element; virt-manager should offer a minimal choice between
BIOS vs. UEFI.
Users who are hard-core enough to hack the domain XML by hand are
expected to provide good values.
The problem I'm raising is that it is *not* sufficient to merely
provide good values in the XML here. You can't simply deploy a
custom OVMF file and update your XML, because this code is relying
on values in the libvirtd.conf configuration file.
If the domain XML spells out both loader and nvram, then both should
be updated manually by the user (if the VM's old nvram is not compatible
with the new loader). This would include the user either instantiating
the new varstore for the VM, or removing the nvram element (so that
the new default template can take effect).
If the domain XML doesn't spell out nvram (either genuinely, or
because the user removed that element, see above), then yes, you need to
edit /etc/libvirt/qemu.conf.
I don't see a problem with that. You won't keep installing OVMF_CODE.fd
files in random locations in the host filesystem. You might be
developing OVMF and install various ad-hoc builds, but those would go to
the same location (same pathname), hence it would have to be added only
once to the qemu.conf file.
I wonder if we should have explicitly record the template in the
XML too. eg
loader.../loader
nvram template=../nvram
If no template attribute is given, then don't try to automatically
create the nvram file at all.
I don't see how this would help with anything. If the template is not
provided, then you immediately branch to the failure case that you don't
seem to like above (ie. no varstore file exists for the guest).
It helps because it allows you to setup custom OVMF builds without
having to edit libvirtd.conf - the information that would go into
the libvirtd.conf file is instead provided directly in the XML.
That information would only be useful if the nvram element had the
@template argument, but no text contents. Meaning for the VM, create my
personal varstore instance from this template if I lose my personal
varstore instance.
And, if the user *wants* to provde a varstore template, then the source
of *that* bit of information is precisely the same as the one of the
loader and nvram elements: the user himself. If the user can be
trusted (expected) to provide a valid varstore template in
nvram/@template, then he can be trusted (expected) just the same to pick
a firmware binary that is listed in the config file.
Nope, that's giving two different experiances. If you use the default
OVMF file, then you don't need to care about manually setting up the
NVRAM store for each guest becaue libvirt can copy the template. If
you use a custom OVMF file apps have to manually setup NVRAM store
per guest.
Not if you edit the nvram=[] map in /etc/libvirt/qemu.conf.
This sucks. The solution of including the nvram template
in the XML allows us to have a consistent experiance where libvirt
is always able to automatically create the per-guest NVRAM file.
If you include the varstore template *only* in the VM-specific domain
XML file, then one very important use case will be broken: creating
brand new VMs with UEFI firmware. Since the domain XML doesn't exist yet
for the virtual machine, there's simply no place to look up the location
of the varstore template in, even if you are trying to use the
system-wide OVMF installation. Users will have to provide the pathname
of the default varstore.
The
[libvirt] [PATCH 2/3] virDriverLoadModule: Honor libvirt func name tranlsation
There's this unwritten rule in libvirt that vir_function is translated
into virFunction when needed (e.g. in remote protocol definition,
python, ...). Up till now we ignored such translation in driver module
loading and did fine. Well, we didn't have any module with an
underscore in its name. But this will change in next commit. The
problem is, once an a module is dlopen()-ed, we derive register
function name from its name. So instead of driver_subdriverRegister
do some magic to turn that into driverSubdriverRegister.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/driver.c | 19 +--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/src/driver.c b/src/driver.c
index 9e3a2eb..71569e6 100644
--- a/src/driver.c
+++ b/src/driver.c
@@ -23,6 +23,7 @@
#include config.h
#include unistd.h
+#include c-ctype.h
#include driver.h
#include viralloc.h
@@ -45,7 +46,8 @@ VIR_LOG_INIT(driver);
void *
virDriverLoadModule(const char *name)
{
-char *modfile = NULL, *regfunc = NULL;
+char *modfile = NULL, *regfunc = NULL, *fixedname = NULL;
+char *tmp;
void *handle = NULL;
int (*regsym)(void);
@@ -72,7 +74,18 @@ virDriverLoadModule(const char *name)
goto cleanup;
}
-if (virAsprintfQuiet(regfunc, %sRegister, name) 0) {
+if (VIR_STRDUP_QUIET(fixedname, name) 0) {
+VIR_ERROR(_(out of memory));
+goto cleanup;
+}
+
+/* convert something_like_this into somethingLikeThis */
+while ((tmp = strchr(fixedname, '_'))) {
+memmove(tmp, tmp + 1, strlen(tmp));
+*tmp = c_toupper(*tmp);
+}
+
+if (virAsprintfQuiet(regfunc, %sRegister, fixedname) 0) {
goto cleanup;
}
@@ -89,11 +102,13 @@ virDriverLoadModule(const char *name)
VIR_FREE(modfile);
VIR_FREE(regfunc);
+VIR_FREE(fixedname);
return handle;
cleanup:
VIR_FREE(modfile);
VIR_FREE(regfunc);
+VIR_FREE(fixedname);
if (handle)
dlclose(handle);
return NULL;
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] libvirt: lxc: Add Get/Set vcpus for lxc
1.Add function to get vcpu count for lxc(vcpucount)
2.Add function to set vcpu count for lxc(setvcpus)
Signed-off-by: Li Yang liyang.f...@cn.fujitsu.com
---
src/lxc/lxc_driver.c | 159 ++
1 files changed, 159 insertions(+), 0 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index 4741632..4df0738 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -5617,6 +5617,162 @@ lxcDomainGetMetadata(virDomainPtr dom,
return ret;
}
+static int
+lxcDomainSetVcpusFlags(virDomainPtr dom, unsigned int nvcpus,
+unsigned int flags)
+{
+virLXCDriverPtr driver = dom-conn-privateData;
+virDomainObjPtr vm = NULL;
+virDomainDefPtr persistentDef;
+int ret = -1;
+bool maximum;
+unsigned int maxvcpus = 0;
+virLXCDriverConfigPtr cfg = NULL;
+virCapsPtr caps = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG |
+ VIR_DOMAIN_VCPU_MAXIMUM |
+ VIR_DOMAIN_VCPU_GUEST, -1);
+
+if (!nvcpus || (unsigned short) nvcpus != nvcpus) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(argument out of range: %d), nvcpus);
+return -1;
+}
+
+if (!(vm = lxcDomObjFromDomain(dom)))
+goto cleanup;
+
+cfg = virLXCDriverGetConfig(driver);
+
+if (virDomainSetVcpusFlagsEnsureACL(dom-conn, vm-def, flags) 0)
+goto cleanup;
+
+if (!(caps = virLXCDriverGetCapabilities(driver, false)))
+goto cleanup;
+
+maximum = (flags VIR_DOMAIN_VCPU_MAXIMUM) != 0;
+flags = ~VIR_DOMAIN_VCPU_MAXIMUM;
+
+if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt, vm, flags,
+persistentDef) 0)
+goto cleanup;
+
+/* MAXIMUM cannot be mixed with LIVE. */
+if (maximum (flags VIR_DOMAIN_AFFECT_LIVE)) {
+virReportError(VIR_ERR_INVALID_ARG, %s,
+ _(cannot adjust maximum on running domain));
+goto cleanup;
+}
+
+if (flags VIR_DOMAIN_AFFECT_LIVE)
+maxvcpus = vm-def-maxvcpus;
+if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+if (!maxvcpus || maxvcpus persistentDef-maxvcpus)
+maxvcpus = persistentDef-maxvcpus;
+}
+if (!maximum nvcpus maxvcpus) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _(requested vcpus is greater than max allowable
+ vcpus for the domain: %d %d),
+ nvcpus, maxvcpus);
+goto cleanup;
+}
+
+if (flags VIR_DOMAIN_VCPU_GUEST) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(changing of vCPU count isn't supported
+ via guest agent));
+goto cleanup;
+} else {
+if (flags VIR_DOMAIN_AFFECT_LIVE) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(Cannot hotplug vCPUS for LXC hypervisor));
+goto cleanup;
+}
+
+if (flags VIR_DOMAIN_AFFECT_CONFIG) {
+if (maximum) {
+persistentDef-maxvcpus = nvcpus;
+if (nvcpus persistentDef-vcpus)
+persistentDef-vcpus = nvcpus;
+} else {
+persistentDef-vcpus = nvcpus;
+}
+
+if (virDomainSaveConfig(cfg-configDir, persistentDef) 0)
+goto cleanup;
+}
+}
+
+ret = 0;
+
+ cleanup:
+if (vm)
+virObjectUnlock(vm);
+virObjectUnref(caps);
+virObjectUnref(cfg);
+return ret;
+}
+
+
+static int
+lxcDomainSetVcpus(virDomainPtr dom, unsigned int nvcpus)
+{
+return lxcDomainSetVcpusFlags(dom, nvcpus, VIR_DOMAIN_AFFECT_LIVE);
+}
+
+
+static int
+lxcDomainGetVcpusFlags(virDomainPtr dom, unsigned int flags)
+{
+virLXCDriverPtr driver = dom-conn-privateData;
+virDomainObjPtr vm;
+virDomainDefPtr def;
+int ret = -1;
+virCapsPtr caps = NULL;
+
+virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
+ VIR_DOMAIN_AFFECT_CONFIG |
+ VIR_DOMAIN_VCPU_MAXIMUM |
+ VIR_DOMAIN_VCPU_GUEST, -1);
+
+if (!(vm = lxcDomObjFromDomain(dom)))
+return -1;
+
+if (virDomainGetVcpusFlagsEnsureACL(dom-conn, vm-def, flags) 0)
+goto cleanup;
+
+if (!(caps = virLXCDriverGetCapabilities(driver, false)))
+goto cleanup;
+
+if (virDomainLiveConfigHelperMethod(caps, driver-xmlopt,
+vm, flags, def) 0)
+goto cleanup;
+
+if (flags VIR_DOMAIN_AFFECT_LIVE)
+def = vm-def;
+
+if (flags VIR_DOMAIN_VCPU_GUEST) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(vCPU count cannot be provided by the guest agent
+ for LXC hypervisor));
+goto cleanup;
+} else {
+if (flags VIR_DOMAIN_VCPU_MAXIMUM)
+
Re: [libvirt] [PATCH v4 3/3] qemu: Automatically create NVRAM store
On Fri, Aug 22, 2014 at 11:38:06AM +0200, Laszlo Ersek wrote: On 08/22/14 10:54, Daniel P. Berrange wrote: On Fri, Aug 22, 2014 at 10:27:29AM +0200, Laszlo Ersek wrote: On 08/21/14 11:05, Daniel P. Berrange wrote: So the user has the ability to specify a arbitrary BIOS in the XML, but unless it matches one of the ones listed in the libvirt config they aren't going to be able to start the guest. What can we do about this, as it doesn't really seem like a great position to be in. I disagree. Users who use virt-manager (for which patches still have to be written, to expose this feature) won't put arbitrary strings in the loader element; virt-manager should offer a minimal choice between BIOS vs. UEFI. Users who are hard-core enough to hack the domain XML by hand are expected to provide good values. The problem I'm raising is that it is *not* sufficient to merely provide good values in the XML here. You can't simply deploy a custom OVMF file and update your XML, because this code is relying on values in the libvirtd.conf configuration file. If the domain XML spells out both loader and nvram, then both should be updated manually by the user (if the VM's old nvram is not compatible with the new loader). This would include the user either instantiating the new varstore for the VM, or removing the nvram element (so that the new default template can take effect). If the domain XML doesn't spell out nvram (either genuinely, or because the user removed that element, see above), then yes, you need to edit /etc/libvirt/qemu.conf. I don't see a problem with that. You won't keep installing OVMF_CODE.fd files in random locations in the host filesystem. You might be developing OVMF and install various ad-hoc builds, but those would go to the same location (same pathname), hence it would have to be added only once to the qemu.conf file. Well I do see a problem with editing qemu.conf for this, particularly when there is a very straightforward way to avoid that need which I have outlined here. It is crazy to force these extra hoops onto people 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] util: compare floor attribute in virNetDevBandwidthEqual
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1064770
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/util/virnetdevbandwidth.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 1e00116..5fa231a 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2009-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
@@ -342,6 +342,7 @@ virNetDevBandwidthEqual(virNetDevBandwidthPtr a,
if (a-in-average != b-in-average ||
a-in-peak != b-in-peak ||
+a-in-floor != b-in-floor ||
a-in-burst != b-in-burst)
return false;
} else if (b-in) {
@@ -355,6 +356,7 @@ virNetDevBandwidthEqual(virNetDevBandwidthPtr a,
if (a-out-average != b-out-average ||
a-out-peak != b-out-peak ||
+a-out-floor != b-out-floor ||
a-out-burst != b-out-burst)
return false;
} else if (b-out) {
--
2.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 3/3] qemu: Automatically create NVRAM store
On 08/22/14 11:56, Daniel P. Berrange wrote: On Fri, Aug 22, 2014 at 11:38:06AM +0200, Laszlo Ersek wrote: On 08/22/14 10:54, Daniel P. Berrange wrote: On Fri, Aug 22, 2014 at 10:27:29AM +0200, Laszlo Ersek wrote: On 08/21/14 11:05, Daniel P. Berrange wrote: So the user has the ability to specify a arbitrary BIOS in the XML, but unless it matches one of the ones listed in the libvirt config they aren't going to be able to start the guest. What can we do about this, as it doesn't really seem like a great position to be in. I disagree. Users who use virt-manager (for which patches still have to be written, to expose this feature) won't put arbitrary strings in the loader element; virt-manager should offer a minimal choice between BIOS vs. UEFI. Users who are hard-core enough to hack the domain XML by hand are expected to provide good values. The problem I'm raising is that it is *not* sufficient to merely provide good values in the XML here. You can't simply deploy a custom OVMF file and update your XML, because this code is relying on values in the libvirtd.conf configuration file. If the domain XML spells out both loader and nvram, then both should be updated manually by the user (if the VM's old nvram is not compatible with the new loader). This would include the user either instantiating the new varstore for the VM, or removing the nvram element (so that the new default template can take effect). If the domain XML doesn't spell out nvram (either genuinely, or because the user removed that element, see above), then yes, you need to edit /etc/libvirt/qemu.conf. I don't see a problem with that. You won't keep installing OVMF_CODE.fd files in random locations in the host filesystem. You might be developing OVMF and install various ad-hoc builds, but those would go to the same location (same pathname), hence it would have to be added only once to the qemu.conf file. Well I do see a problem with editing qemu.conf for this, particularly when there is a very straightforward way to avoid that need which I have outlined here. It is crazy to force these extra hoops onto people OK. But, if you don't provide a default map in some central config file, for at least the system-wide OVMF installation(s), how do you save people from the exact same burden (== manual varstore instantiation), when they try to create a brand new UEFI virtual machine that uses one of the system-wide OVMF filesets? Libvirt won't know where to copy the varstore from. What you outlined is not straightforward at all for this case. Since the VM is just being created, it has no nvram/@template attribute (because the domain XML is being composed, it doesn't exist yet). The v4 patchset addresses the common case and allows the special use case to remain hard. As far as I understand, your proposal breaks the common use case (== the main goal of this patchset), namely that users can just say I have all the necessary distro packages installed, now give me a new OVMF VM. Thanks Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] util: compare floor attribute in virNetDevBandwidthEqual
On 22.08.2014 12:10, Martin Kletzander wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1064770
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/util/virnetdevbandwidth.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 1e00116..5fa231a 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2009-2013 Red Hat, Inc.
+ * Copyright (C) 2009-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
@@ -342,6 +342,7 @@ virNetDevBandwidthEqual(virNetDevBandwidthPtr a,
if (a-in-average != b-in-average ||
a-in-peak != b-in-peak ||
+a-in-floor != b-in-floor ||
a-in-burst != b-in-burst)
return false;
} else if (b-in) {
@@ -355,6 +356,7 @@ virNetDevBandwidthEqual(virNetDevBandwidthPtr a,
if (a-out-average != b-out-average ||
a-out-peak != b-out-peak ||
+a-out-floor != b-out-floor ||
a-out-burst != b-out-burst)
return false;
} else if (b-out) {
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 3/3] qemu: Automatically create NVRAM store
On Fri, Aug 22, 2014 at 12:12:52PM +0200, Laszlo Ersek wrote: On 08/22/14 11:56, Daniel P. Berrange wrote: On Fri, Aug 22, 2014 at 11:38:06AM +0200, Laszlo Ersek wrote: On 08/22/14 10:54, Daniel P. Berrange wrote: On Fri, Aug 22, 2014 at 10:27:29AM +0200, Laszlo Ersek wrote: On 08/21/14 11:05, Daniel P. Berrange wrote: So the user has the ability to specify a arbitrary BIOS in the XML, but unless it matches one of the ones listed in the libvirt config they aren't going to be able to start the guest. What can we do about this, as it doesn't really seem like a great position to be in. I disagree. Users who use virt-manager (for which patches still have to be written, to expose this feature) won't put arbitrary strings in the loader element; virt-manager should offer a minimal choice between BIOS vs. UEFI. Users who are hard-core enough to hack the domain XML by hand are expected to provide good values. The problem I'm raising is that it is *not* sufficient to merely provide good values in the XML here. You can't simply deploy a custom OVMF file and update your XML, because this code is relying on values in the libvirtd.conf configuration file. If the domain XML spells out both loader and nvram, then both should be updated manually by the user (if the VM's old nvram is not compatible with the new loader). This would include the user either instantiating the new varstore for the VM, or removing the nvram element (so that the new default template can take effect). If the domain XML doesn't spell out nvram (either genuinely, or because the user removed that element, see above), then yes, you need to edit /etc/libvirt/qemu.conf. I don't see a problem with that. You won't keep installing OVMF_CODE.fd files in random locations in the host filesystem. You might be developing OVMF and install various ad-hoc builds, but those would go to the same location (same pathname), hence it would have to be added only once to the qemu.conf file. Well I do see a problem with editing qemu.conf for this, particularly when there is a very straightforward way to avoid that need which I have outlined here. It is crazy to force these extra hoops onto people OK. But, if you don't provide a default map in some central config file, for at least the system-wide OVMF installation(s), how do you save people from the exact same burden (== manual varstore instantiation), when they try to create a brand new UEFI virtual machine that uses one of the system-wide OVMF filesets? Libvirt won't know where to copy the varstore from. Having a default map in libvirt and in qemu.conf is still acceptable for the common case. I just want to make sure that if the user wants to provide a non-default BIOS in loader they can still get the NVRAM automatically created from a template, without having to edit qemu.conf and restart libvirtd. 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] virsh: Fix help info for freepages
On 21.08.2014 05:19, Li Yang wrote:
Signed-off-by: Li Yang liyang.f...@cn.fujitsu.com
---
tools/virsh-host.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-host.c b/tools/virsh-host.c
index ae14311..ad821b3 100644
--- a/tools/virsh-host.c
+++ b/tools/virsh-host.c
@@ -263,10 +263,10 @@ cmdFreecell(vshControl *ctl, const vshCmd *cmd)
*/
static const vshCmdInfo info_freepages[] = {
{.name = help,
- .data = N_(NUMA free memory)
+ .data = N_(NUMA free pages)
},
{.name = desc,
- .data = N_(display available free memory for the NUMA cell.)
+ .data = N_(display available free pages for the NUMA cell.)
},
{.name = NULL}
};
ACKed pushed.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 3/3] qemu: Automatically create NVRAM store
On 08/22/14 12:24, Daniel P. Berrange wrote: On Fri, Aug 22, 2014 at 12:12:52PM +0200, Laszlo Ersek wrote: On 08/22/14 11:56, Daniel P. Berrange wrote: On Fri, Aug 22, 2014 at 11:38:06AM +0200, Laszlo Ersek wrote: On 08/22/14 10:54, Daniel P. Berrange wrote: On Fri, Aug 22, 2014 at 10:27:29AM +0200, Laszlo Ersek wrote: On 08/21/14 11:05, Daniel P. Berrange wrote: So the user has the ability to specify a arbitrary BIOS in the XML, but unless it matches one of the ones listed in the libvirt config they aren't going to be able to start the guest. What can we do about this, as it doesn't really seem like a great position to be in. I disagree. Users who use virt-manager (for which patches still have to be written, to expose this feature) won't put arbitrary strings in the loader element; virt-manager should offer a minimal choice between BIOS vs. UEFI. Users who are hard-core enough to hack the domain XML by hand are expected to provide good values. The problem I'm raising is that it is *not* sufficient to merely provide good values in the XML here. You can't simply deploy a custom OVMF file and update your XML, because this code is relying on values in the libvirtd.conf configuration file. If the domain XML spells out both loader and nvram, then both should be updated manually by the user (if the VM's old nvram is not compatible with the new loader). This would include the user either instantiating the new varstore for the VM, or removing the nvram element (so that the new default template can take effect). If the domain XML doesn't spell out nvram (either genuinely, or because the user removed that element, see above), then yes, you need to edit /etc/libvirt/qemu.conf. I don't see a problem with that. You won't keep installing OVMF_CODE.fd files in random locations in the host filesystem. You might be developing OVMF and install various ad-hoc builds, but those would go to the same location (same pathname), hence it would have to be added only once to the qemu.conf file. Well I do see a problem with editing qemu.conf for this, particularly when there is a very straightforward way to avoid that need which I have outlined here. It is crazy to force these extra hoops onto people OK. But, if you don't provide a default map in some central config file, for at least the system-wide OVMF installation(s), how do you save people from the exact same burden (== manual varstore instantiation), when they try to create a brand new UEFI virtual machine that uses one of the system-wide OVMF filesets? Libvirt won't know where to copy the varstore from. Having a default map in libvirt and in qemu.conf is still acceptable for the common case. I just want to make sure that if the user wants to provide a non-default BIOS in loader they can still get the NVRAM automatically created from a template, without having to edit qemu.conf and restart libvirtd. Michal, can you please incorporate this? I guess the priorities would be: - if nvram has text contents, just use that. (same as now) - else, if nvram has @template, copy from there. (new) - otherwise, consult the map. (same as now) Thanks, Laszlo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 0/6] ivshmem support
The following patches are an implementation of a new shmem device to support ivshmem in libvirt. Any feedback is welcome. Changes since RFC: - replace ivshmem by a more generic term shmem - remove role attribute - libvirt is able to start and stop an ivshmem server - update xml format from: ivshmem use_server='yes' role='master' source file='/tmp/socket-ivshmem0'/ size unit='M'32/size msi vectors='32' ioeventfd='on'/ /ivshmem to: shmem name='shmem0' model='ivshmem' server path='/tmp/sockect-ivshem0' start='yes'/ size unit='M'32/size msi vectors='32' ioeventfd='on'/ /shmem Maxime Leroy (6): doc: schema: Add documentation for the shmem support conf: Parse and format shmem device XML qemu: Add cap flag QEMU_CAPS_IVSHMEM qemu: Build command line for ivshmem device tests: Add tests for ivshmem device handling ivshmem: add start param to server attribute configure.ac | 4 + docs/formatdomain.html.in| 71 ++ docs/schemas/domaincommon.rng| 52 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/conf/domain_conf.c | 264 ++- src/conf/domain_conf.h | 42 src/libvirt_private.syms | 7 + src/qemu/qemu_capabilities.c | 3 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 112 +- src/qemu/qemu_command.h | 4 + src/qemu/qemu_hotplug.c | 1 + src/qemu/qemu_process.c | 10 + src/util/virivshmemserver.c | 141 src/util/virivshmemserver.h | 28 +++ tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps| 1 + tests/qemuhelptest.c | 17 +- tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args | 10 + tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml | 35 +++ tests/qemuxml2argvtest.c | 3 + tests/qemuxml2xmltest.c | 2 + 27 files changed, 808 insertions(+), 7 deletions(-) create mode 100644 src/util/virivshmemserver.c create mode 100644 src/util/virivshmemserver.h create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 1/6] doc: schema: Add documentation for the shmem support
This patch documents XML elements used for support of shmem devices. Considering there could be other shared memory related devices, this patch introduces a general device model, called shmem device. Currenly ivshmem is the only shared memory model supported. About ivshmem, please see the following documentation: http://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/ivshmem_device_spec.txt (Ivshmem documentation for qemu will be updated soon: http://lists.gnu.org/archive/html/qemu-devel/2014-08/msg01244.html) In the devices section in the domain XML users may specify: - For ivshmem device using an ivshmem server: shmem name='shmem0' model='ivshmem' source file='/tmp/socket-ivshmem0'/ size unit='M'32/size msi vectors='32' ioeventfd='on'/ /shmem - For ivshmem device not using an ivshmem server: shmem name='ivshmem1' model='ivshmem' size unit='M'32/size /shmem Signed-off-by: Maxime Leroy maxime.le...@6wind.com --- docs/formatdomain.html.in | 68 +++ docs/schemas/domaincommon.rng | 49 +++ 2 files changed, 117 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index ed17389..c7644bc 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5422,6 +5422,74 @@ qemu-kvm -net nic,model=? /dev/null /dd /dl +h4a name=elementsShmemShmem device/a/h4 + +p + A shmem (i.e. shared memory) device allows to share a memory + region between different virtual machines and the host. +/p + +pre + ... + lt;devicesgt; +lt;shmem name='shmem0' model='ivshmem'/gt; +lt;shmem name='shmem1' model='ivshmem'/gt; + lt;/devicesgt; + .../pre + +dl + dtcodeshmem/code/dt + dd + The codeshmem/code element has one mandatory attribute, + codename/code to identify the shared memory. The optional element + codemodel/code specify the type of shared memory. Only one mode of + shared-memory is currently supported: codeivshmem/code. + /dd +/dl + +h5a name=elementsIvshmemIvshmem model/a/h5 + +p + An ivshmem (i.e. Inter-VM shared memory) device allows to share a memory + region (created on the host via the POSIX shared memory API) between + multiple QEMU processes running different guests. + For more information, please see the + a href=http://git.qemu.org/?p=qemu.git;a=blob;f=docs/specs/ivshmem_device_spec.txt; +ivshmem documentation of qemu/a. +/p + +pre + ... + lt;devicesgt; +lt;shmem name='shmem0' model='ivshmem'gt; + lt;server path='/tmp/socket-shmem0'/gt; + lt;size unit='M'gt;32lt;/sizegt; + lt;msi vectors='32' ioeventfd='on'/gt; +lt;/shmemgt; +lt;shmem name='shmem1' model='ivshmem'gt; + lt;size unit='M'gt;32lt;/sizegt; +lt;/shmemgt; + lt;/devicesgt; + .../pre + + dl +dtcodeserver/code/dt +ddThe optional codeserver/code element can be used to configure an + ivshmem device connected to the ivshmem server via a unix socket. The + codepath/code attribute specifies the path to the unix socket. +/dd +dtcodesize/code/dt +ddThe optional codesize/code element specifies the size of the shared memory. +/dd +dtcodemsi/code/dt +ddThe optional codemsi/code element allows to enable MSI interrupts. + This option can only be used with the codeserver/code attribute. + The codevectors/code attribute can be used to specify the number of + interrupt vectors. The codeioeventd/code attribute allows to enable + ioeventfd. +/dd + /dl + h3a name=seclabelSecurity label/a/h3 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 033f2f6..64abf2b 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3221,6 +3221,54 @@ /optional /element /define + + define name=shmem +element name=shmem + attribute name=name/ + optional +ref name=ivshmem/ + /optional + optional +ref name=alias/ + /optional + optional +ref name=address/ + /optional +/element + /define + + define name=ivshmem + group + attribute name=model + valueivshmem/value + /attribute + interleave + optional + element name=server + attribute name=path/ + /element + /optional + optional + element name=msi + optional + ref name=ioeventfd/ + /optional + optional + attribute name=vectors + ref name=unsignedInt/ + /attribute + /optional + /element + /optional + optional + element name=size + ref name=scaledInteger/ + /element + /optional + /interleave + /group + /define + define name=memballoon element name=memballoon attribute name=model @@ -3807,6
[libvirt] [PATCH v1 6/6] ivshmem: add start param to server attribute
With the new start param, we are able to start the ivshmem-server. With this xml: shmem name='ivshmem0' model='ivshmem' server path='/tmp/ivshmem0.sock' start='yes'/ size unit='M'32/size msi vectors='32' ioeventfd='on'/ /shmem Libvirt will execute an ivshmem-server: /usr/bin/ivshmem-server -m ivshmem0 -S /tmp/ivshmem0.sock \ -p /var/run/libvirt/ivshmem-server/ivshd-ivshmem0.pid -n 32 Signed-off-by: Maxime Leroy maxime.le...@6wind.com --- configure.ac| 4 + docs/formatdomain.html.in | 5 +- docs/schemas/domaincommon.rng | 3 + po/POTFILES.in | 1 + src/Makefile.am | 1 + src/conf/domain_conf.c | 19 +++- src/conf/domain_conf.h | 1 + src/libvirt_private.syms| 5 + src/qemu/qemu_command.c | 7 ++ src/qemu/qemu_process.c | 10 ++ src/util/virivshmemserver.c | 141 src/util/virivshmemserver.h | 28 + tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml | 2 +- 13 files changed, 223 insertions(+), 4 deletions(-) create mode 100644 src/util/virivshmemserver.c create mode 100644 src/util/virivshmemserver.h diff --git a/configure.ac b/configure.ac index f93c6c2..6b525b9 100644 --- a/configure.ac +++ b/configure.ac @@ -431,6 +431,8 @@ AC_PATH_PROG([SCRUB], [scrub], [scrub], [/sbin:/usr/sbin:/usr/local/sbin:$PATH]) AC_PATH_PROG([ADDR2LINE], [addr2line], [addr2line], [/sbin:/usr/bin:/usr/sbin:/usr/local/sbin:$PATH]) +AC_PATH_PROG([IVSHMEMSERVER], [ivshmem-server], [ivshmem-server], + [/usr/bin:/usr/local/bin:$PATH]) AC_DEFINE_UNQUOTED([DMIDECODE],[$DMIDECODE], [Location or name of the dmidecode program]) @@ -463,6 +465,8 @@ AC_DEFINE_UNQUOTED([SCRUB],[$SCRUB], [Location or name of the scrub program (for wiping algorithms)]) AC_DEFINE_UNQUOTED([ADDR2LINE],[$ADDR2LINE], [Location of addr2line program]) +AC_DEFINE_UNQUOTED([IVSHMEMSERVER], [$IVSHMEMSERVER], + [Location or name of ivshmem-server program]) dnl Specific dir for HTML output ? AC_ARG_WITH([html-dir], [AS_HELP_STRING([--with-html-dir=path], diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index c7644bc..df231d8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -5462,7 +5462,7 @@ qemu-kvm -net nic,model=? /dev/null ... lt;devicesgt; lt;shmem name='shmem0' model='ivshmem'gt; - lt;server path='/tmp/socket-shmem0'/gt; + lt;server path='/tmp/socket-shmem0' start='yes'/gt; lt;size unit='M'gt;32lt;/sizegt; lt;msi vectors='32' ioeventfd='on'/gt; lt;/shmemgt; @@ -5477,6 +5477,9 @@ qemu-kvm -net nic,model=? /dev/null ddThe optional codeserver/code element can be used to configure an ivshmem device connected to the ivshmem server via a unix socket. The codepath/code attribute specifies the path to the unix socket. + The codestart/code attribute specifies if the libvirt must start + the ivshmem-server. By default, libvirt expects that the ivshmem-server is + already running on the host. /dd dtcodesize/code/dt ddThe optional codesize/code element specifies the size of the shared memory. diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 64abf2b..a601747 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -3246,6 +3246,9 @@ optional element name=server attribute name=path/ + optional + attribute name=start/ + /optional /element /optional optional diff --git a/po/POTFILES.in b/po/POTFILES.in index f17b35f..7d90517 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -177,6 +177,7 @@ src/util/viridentity.c src/util/virinitctl.c src/util/viriptables.c src/util/viriscsi.c +src/util/virivshmemserver.c src/util/virjson.c src/util/virkeyfile.c src/util/virlockspace.c diff --git a/src/Makefile.am b/src/Makefile.am index 538530e..00e1ccf 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -118,6 +118,7 @@ UTIL_SOURCES = \ util/virinitctl.c util/virinitctl.h \ util/viriptables.c util/viriptables.h \ util/viriscsi.c util/viriscsi.h \ + util/virivshmemserver.c util/virivshmemserver.h \ util/virjson.c util/virjson.h \ util/virkeycode.c util/virkeycode.h \ util/virkeyfile.c util/virkeyfile.h \ diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 08d653a..224b367 100644 --- a/src/conf/domain_conf.c +++
[libvirt] [PATCH v1 2/6] conf: Parse and format shmem device XML
This patch adds configuration support for the shmem device
as described in the schema in the previous patch.
Signed-off-by: Maxime Leroy maxime.le...@6wind.com
---
src/conf/domain_conf.c | 249 ++-
src/conf/domain_conf.h | 41
src/libvirt_private.syms | 2 +
3 files changed, 291 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9557020..08d653a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -234,7 +234,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
chr,
memballoon,
nvram,
- rng)
+ rng,
+ shmem)
VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
none,
@@ -759,6 +760,9 @@ VIR_ENUM_IMPL(virDomainDiskMirrorState,
VIR_DOMAIN_DISK_MIRROR_STATE_LAST,
abort,
pivot)
+VIR_ENUM_IMPL(virDomainShmemModel, VIR_DOMAIN_SHMEM_MODEL_LAST,
+ ivshmem)
+
/* Internal mapping: subset of block job types that can be present in
* mirror XML (remaining types are not two-phase). */
VIR_ENUM_DECL(virDomainBlockJob)
@@ -1692,6 +1696,26 @@ void virDomainWatchdogDefFree(virDomainWatchdogDefPtr
def)
VIR_FREE(def);
}
+void virDomainShmemDefFree(virDomainShmemDefPtr def)
+{
+if (!def)
+return;
+
+switch (def-model) {
+case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
+VIR_FREE(def-data.ivshmem.server.path);
+break;
+default:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unexpected shmem model %d), def-model);
+}
+
+virDomainDeviceInfoClear(def-info);
+
+VIR_FREE(def-name);
+VIR_FREE(def);
+}
+
void virDomainVideoDefFree(virDomainVideoDefPtr def)
{
if (!def)
@@ -1893,6 +1917,9 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
case VIR_DOMAIN_DEVICE_NVRAM:
virDomainNVRAMDefFree(def-data.nvram);
break;
+case VIR_DOMAIN_DEVICE_SHMEM:
+virDomainShmemDefFree(def-data.shmem);
+break;
case VIR_DOMAIN_DEVICE_LAST:
case VIR_DOMAIN_DEVICE_NONE:
break;
@@ -2134,6 +2161,10 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainRedirFilterDefFree(def-redirfilter);
+for (i = 0; i def-nshmems; i++)
+virDomainShmemDefFree(def-shmems[i]);
+VIR_FREE(def-shmems);
+
if (def-namespaceData def-ns.free)
(def-ns.free)(def-namespaceData);
@@ -2568,6 +2599,8 @@ virDomainDeviceGetInfo(virDomainDeviceDefPtr device)
return device-data.memballoon-info;
case VIR_DOMAIN_DEVICE_NVRAM:
return device-data.nvram-info;
+case VIR_DOMAIN_DEVICE_SHMEM:
+return device-data.shmem-info;
case VIR_DOMAIN_DEVICE_RNG:
return device-data.rng-info;
@@ -2783,6 +2816,12 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
if (cb(def, device, def-hubs[i]-info, opaque) 0)
return -1;
}
+device.type = VIR_DOMAIN_DEVICE_SHMEM;
+for (i = 0; i def-nshmems; i++) {
+device.data.shmem = def-shmems[i];
+if (cb(def, device, def-shmems[i]-info, opaque) 0)
+return -1;
+}
/* This switch statement is here to trigger compiler warning when adding
* a new device type. When you are adding a new field to the switch you
@@ -2809,6 +2848,7 @@ virDomainDeviceInfoIterateInternal(virDomainDefPtr def,
case VIR_DOMAIN_DEVICE_CHR:
case VIR_DOMAIN_DEVICE_MEMBALLOON:
case VIR_DOMAIN_DEVICE_NVRAM:
+case VIR_DOMAIN_DEVICE_SHMEM:
case VIR_DOMAIN_DEVICE_LAST:
case VIR_DOMAIN_DEVICE_RNG:
break;
@@ -9462,6 +9502,135 @@ virDomainNVRAMDefParseXML(xmlNodePtr node,
return NULL;
}
+static int
+virDomainIvshmemDefParseXML(xmlNodePtr node,
+xmlXPathContextPtr ctxt,
+virDomainIvshmemDefPtr def)
+{
+char *ioeventfd = NULL;
+char *vectors = NULL;
+xmlNodePtr cur;
+xmlNodePtr save = ctxt-node;
+int ret;
+
+cur = node-children;
+while (cur != NULL) {
+if (cur-type == XML_ELEMENT_NODE) {
+if (xmlStrEqual(cur-name, BAD_CAST server)) {
+def-server.enabled = true;
+if (!(def-server.path = virXMLPropString(cur, path))) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(cannot parse server 'path'
attribute));
+goto error;
+}
+} else if (xmlStrEqual(cur-name, BAD_CAST size)) {
+if (virDomainParseScaledValue(./size[1], ctxt,
+ def-size, 1,
+ ULLONG_MAX, true) 0) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(cannot parse size attribute));
+goto
[libvirt] [PATCH v1 4/6] qemu: Build command line for ivshmem device
This patch implements support for the ivshmem device
in QEMU.
Example from this xml:
shmem name='ivshmem0' model='ivshmem'
server path='/tmp/socket-ivshmem0'/
size unit='M'32/size
msi vectors='32' ioeventfd='on'/
/shmem
The following QEMU line is built:
-device ivshmem,size=32m,vectors=32,chardev=charshmem0,msi=on,
ioeventfd=on,role=master
-chardev socket,path=/tmp/socket-ivshmem0,id=charshmem0
Note: PCI hotpluging is not implemented.
Signed-off-by: Maxime Leroy maxime.le...@6wind.com
---
src/qemu/qemu_command.c | 105 +++-
src/qemu/qemu_command.h | 4 ++
src/qemu/qemu_hotplug.c | 1 +
3 files changed, 109 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0d7b12d..9fcceae 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1019,6 +1019,10 @@ qemuAssignDeviceAliases(virDomainDefPtr def,
virQEMUCapsPtr qemuCaps)
if (virAsprintf(def-hubs[i]-info.alias, hub%zu, i) 0)
return -1;
}
+for (i = 0; i def-nshmems; i++) {
+if (virAsprintf(def-shmems[i]-info.alias, shmem%zu, i) 0)
+return -1;
+}
for (i = 0; i def-nsmartcards; i++) {
if (virAsprintf(def-smartcards[i]-info.alias, smartcard%zu, i)
0)
return -1;
@@ -5043,6 +5047,100 @@ qemuBuildRedirdevDevStr(virDomainDefPtr def,
return NULL;
}
+static char *
+qemuBuildIvshmemDevStr(virDomainDefPtr def,
+ virDomainShmemDefPtr dev,
+ virQEMUCapsPtr qemuCaps)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+virDomainIvshmemDefPtr ivshmem = dev-data.ivshmem;
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IVSHMEM)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(ivshmem device is not supported by QEMU));
+goto error;
+}
+
+virBufferAddLit(buf, ivshmem);
+if (ivshmem-size)
+virBufferAsprintf(buf, ,size=%llum, ivshmem-size / (1024 * 1024));
+
+if (!ivshmem-server.enabled)
+virBufferAsprintf(buf, ,shm=%s, dev-name);
+else {
+virBufferAsprintf(buf, ,chardev=char%s, dev-info.alias);
+if (ivshmem-msi.enabled) {
+virBufferAddLit(buf, ,msi=on);
+if (ivshmem-msi.vectors)
+virBufferAsprintf(buf, ,vectors=%u, ivshmem-msi.vectors);
+if (ivshmem-msi.ioeventfd)
+virBufferAsprintf(buf, ,ioeventfd=%s,
+
virTristateSwitchTypeToString(ivshmem-msi.ioeventfd));
+}
+}
+
+if (qemuBuildDeviceAddressStr(buf, def, dev-info, qemuCaps) 0)
+goto error;
+
+if (virBufferCheckError(buf) 0)
+goto error;
+
+return virBufferContentAndReset(buf);
+
+ error:
+virBufferFreeAndReset(buf);
+return NULL;
+}
+
+static int
+qemuBuildIvshmemCommandLine(virCommandPtr cmd,
+virDomainDefPtr def,
+virDomainShmemDefPtr dev,
+virQEMUCapsPtr qemuCaps)
+{
+char *devstr;
+virDomainIvshmemDefPtr ivshmem = dev-data.ivshmem;
+
+virCommandAddArg(cmd, -device);
+if (!(devstr = qemuBuildIvshmemDevStr(def, dev, qemuCaps)))
+return -1;
+virCommandAddArg(cmd, devstr);
+VIR_FREE(devstr);
+
+if (ivshmem-server.enabled) {
+virDomainChrSourceDef source;
+
+source.type = VIR_DOMAIN_CHR_TYPE_UNIX;
+source.data.nix.path = ivshmem-server.path;
+source.data.nix.listen = false;
+
+virCommandAddArg(cmd, -chardev);
+if (!(devstr = qemuBuildChrChardevStr(source, dev-info.alias,
+ qemuCaps)))
+return -1;
+virCommandAddArg(cmd, devstr);
+VIR_FREE(devstr);
+}
+
+return 0;
+}
+
+static int
+qemuBuildShmemCommandLine(virCommandPtr cmd,
+ virDomainDefPtr def,
+ virDomainShmemDefPtr dev,
+ virQEMUCapsPtr qemuCaps)
+{
+switch (dev-model) {
+case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
+return qemuBuildIvshmemCommandLine(cmd, def, dev, qemuCaps);
+default:
+virReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Unexpected shem model %d), dev-model);
+}
+return -1;
+}
+
char *
qemuBuildUSBHostdevDevStr(virDomainDefPtr def,
virDomainHostdevDefPtr dev,
@@ -5299,7 +5397,7 @@ qemuBuildSCSIHostdevDevStr(virDomainDefPtr def,
/* This function outputs a -chardev command line option which describes only
the
* host side of the character device */
-static char *
+char *
qemuBuildChrChardevStr(virDomainChrSourceDefPtr dev, const char *alias,
virQEMUCapsPtr qemuCaps)
{
@@ -9332,6 +9430,11 @@ qemuBuildCommandLine(virConnectPtr conn,
}
}
+for (i =
[libvirt] [PATCH v1 5/6] tests: Add tests for ivshmem device handling
Add XML parsing and qemu command line tests for the ivshmem device support. Signed-off-by: Maxime Leroy maxime.le...@6wind.com --- tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args | 10 +++ tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml | 35 tests/qemuxml2argvtest.c | 3 ++ tests/qemuxml2xmltest.c | 2 ++ 4 files changed, 50 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args new file mode 100644 index 000..8a5cc0f --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.args @@ -0,0 +1,10 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \ +/usr/bin/qemu -S \ +-M pc -m 214 -smp 1 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 \ +-device ivshmem,chardev=charshmem0,msi=on,vectors=32,ioeventfd=on \ +-chardev socket,id=charshmem0,path=/tmp/socket-shmem0 \ +-device ivshmem,size=32m,chardev=charshmem1,msi=on,vectors=32 \ +-chardev socket,id=charshmem1,path=/tmp/socket-shmem1 \ +-device ivshmem,size=32m,shm=shmem2,bus=pci.0,addr=0x8 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml new file mode 100644 index 000..7177612 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-ivshmem.xml @@ -0,0 +1,35 @@ +domain type='qemu' + nameQEMUGuest1/name + uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid + memory unit='KiB'219100/memory + currentMemory unit='KiB'219100/currentMemory + vcpu placement='static' cpuset='1-4,8-20,525'1/vcpu + os +type arch='x86_64' machine='pc'hvm/type +boot dev='hd'/ + /os + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu/emulator +controller type='usb' index='0'/ +controller type='ide' index='0'/ +controller type='pci' index='0' model='pci-root'/ +memballoon model='virtio'/ +shmem name='shmem0' model='ivshmem' + server path='/tmp/socket-shmem0'/ + msi vectors='32' ioeventfd='on'/ +/shmem +shmem name='shmem1' model='ivshmem' + server path='/tmp/socket-shmem1'/ + size unit='M'32/size + msi vectors='32'/ +/shmem +shmem name='shmem2' model='ivshmem' + size unit='M'32/size + address type='pci' domain='0x' bus='0x00' slot='0x08' function='0x0'/ +/shmem + /devices +/domain diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 65dc9c7..f91ac36 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1431,6 +1431,9 @@ mymain(void) DO_TEST(panic, QEMU_CAPS_DEVICE_PANIC, QEMU_CAPS_DEVICE, QEMU_CAPS_NODEFCONFIG); +DO_TEST(ivshmem, QEMU_CAPS_PCIDEVICE, +QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_IVSHMEM); + virObjectUnref(driver.config); virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 5941323..4e62fe9 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -388,6 +388,8 @@ mymain(void) DO_TEST_DIFFERENT(numatune-memnode); DO_TEST(numatune-memnode-no-memory); +DO_TEST(ivshmem); + virObjectUnref(driver.caps); virObjectUnref(driver.xmlopt); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 3/6] qemu: Add cap flag QEMU_CAPS_IVSHMEM
Ivshmem is supported by QEMU since 0.13 release.
Signed-off-by: Maxime Leroy maxime.le...@6wind.com
---
src/qemu/qemu_capabilities.c | 3 +++
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_1.2.2-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.3.1-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.4.2-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 +
tests/qemuhelptest.c | 17 -
9 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b758b5a..49e0f0d 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -266,6 +266,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
memory-backend-file,
usb-audio,
rtc-reset-reinjection,
+
+ ivshmem, /* 175 */
);
@@ -1487,6 +1489,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = {
{ memory-backend-ram, QEMU_CAPS_OBJECT_MEMORY_RAM },
{ memory-backend-file, QEMU_CAPS_OBJECT_MEMORY_FILE },
{ usb-audio, QEMU_CAPS_OBJECT_USB_AUDIO },
+{ ivshmem, QEMU_CAPS_DEVICE_IVSHMEM },
};
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index cbd3646..1005a4e 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -214,6 +214,7 @@ typedef enum {
QEMU_CAPS_OBJECT_MEMORY_FILE = 172, /* -object memory-backend-file */
QEMU_CAPS_OBJECT_USB_AUDIO = 173, /* usb-audio device support */
QEMU_CAPS_RTC_RESET_REINJECTION = 174, /* rtc-reset-reinjection monitor
command */
+QEMU_CAPS_DEVICE_IVSHMEM = 175, /* -device ivshmem */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
index c8a379a..f32bc27 100644
--- a/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.2.2-1.caps
@@ -116,4 +116,5 @@
flag name='usb-kbd'/
flag name='host-pci-multidomain'/
flag name='usb-audio'/
+flag name='ivshmem'/
/qemuCaps
diff --git a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
index 4b7651e..f4f0397 100644
--- a/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.3.1-1.caps
@@ -129,4 +129,5 @@
flag name='usb-kbd'/
flag name='host-pci-multidomain'/
flag name='usb-audio'/
+flag name='ivshmem'/
/qemuCaps
diff --git a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
index d146bf9..e6659e4 100644
--- a/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.4.2-1.caps
@@ -130,4 +130,5 @@
flag name='usb-kbd'/
flag name='host-pci-multidomain'/
flag name='usb-audio'/
+flag name='ivshmem'/
/qemuCaps
diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
index 5fa30aa..591cde6 100644
--- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
@@ -138,4 +138,5 @@
flag name='usb-kbd'/
flag name='host-pci-multidomain'/
flag name='usb-audio'/
+flag name='ivshmem'/
/qemuCaps
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
index f364bbf..538a16e 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
@@ -145,4 +145,5 @@
flag name='host-pci-multidomain'/
flag name='msg-timestamp'/
flag name='usb-audio'/
+flag name='ivshmem'/
/qemuCaps
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
index 10ce1b5..b814e8a 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
@@ -144,4 +144,5 @@
flag name='msg-timestamp'/
flag name='numa'/
flag name='usb-audio'/
+flag name='ivshmem'/
/qemuCaps
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 366e36d..ba12cdb 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -518,7 +518,8 @@ mymain(void)
QEMU_CAPS_DEVICE_SCSI_GENERIC,
QEMU_CAPS_DEVICE_USB_KBD,
QEMU_CAPS_DEVICE_USB_STORAGE,
-QEMU_CAPS_HOST_PCI_MULTIDOMAIN);
+QEMU_CAPS_HOST_PCI_MULTIDOMAIN,
+QEMU_CAPS_DEVICE_IVSHMEM);
DO_TEST(qemu-kvm-0.12.1.2-rhel61, 12001, 1, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -745,7 +746,8 @@ mymain(void)
QEMU_CAPS_DEVICE_SCSI_GENERIC,
Re: [libvirt] [PATCH 0/3] storage: zfs: implement download and upload
On 15.08.2014 10:44, Roman Bogorodskiy wrote: Roman Bogorodskiy (3): fdstream: report error if virSetNonBlock fails fdstream: introduce virFDStreamOpenBlockDevice storage: zfs: implement download and upload src/fdstream.c| 30 +++--- src/fdstream.h| 5 + src/libvirt_private.syms | 1 + src/storage/storage_backend.c | 6 -- src/storage/storage_backend_zfs.c | 2 ++ 5 files changed, 35 insertions(+), 9 deletions(-) ACK to all the patches, if you work in Dan's comment in 2/3. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 3/5] Parallels: Change config report errors code.
Wrong error code in config errors reporting was used. Fixed it.
---
src/parallels/parallels_driver.c | 81
1 file changed, 41 insertions(+), 40 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 3806e5e..0373830 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -1532,7 +1532,7 @@ parallelsApplyGraphicsParams(virDomainGraphicsDefPtr
*oldgraphics, int nold,
return 0;
error:
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(changing display parameters is not supported
by parallels driver));
return -1;
@@ -1580,7 +1580,7 @@ parallelsApplySerialParams(virDomainChrDefPtr
*oldserials, int nold,
return 0;
error:
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(changing serial device parameters is
not supported by parallels driver));
return -1;
@@ -1595,7 +1595,7 @@ parallelsApplyVideoParams(parallelsDomObjPtr pdom,
char str_vram[32];
if (nold != 1 || nnew != 1) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Only one video device is
supported by parallels driver));
return -1;
@@ -1604,14 +1604,14 @@ parallelsApplyVideoParams(parallelsDomObjPtr pdom,
old = oldvideos[0];
new = newvideos[0];
if (new-type != VIR_DOMAIN_VIDEO_TYPE_VGA) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Only VGA video device is
supported by parallels driver));
return -1;
}
if (new-heads != 1) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Only one monitor is supported by parallels driver));
return -1;
}
@@ -1621,15 +1621,16 @@ parallelsApplyVideoParams(parallelsDomObjPtr pdom,
old-accel-support2d != new-accel-support2d ||
old-accel-support3d != new-accel-support3d) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Changing video acceleration parameters is
not supported by parallels driver));
return -1;
+
}
if (old-vram != new-vram) {
if (new-vram % (1 10) != 0) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Video RAM size should be multiple of 1Mb.));
return -1;
}
@@ -1659,7 +1660,7 @@ static int parallelsAddHdd(parallelsDomObjPtr pdom,
int format = virDomainDiskGetFormat(disk);
if (format != VIR_STORAGE_FILE_PLOOP) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Invalid disk format: %d), type);
goto cleanup;
}
@@ -1668,7 +1669,7 @@ static int parallelsAddHdd(parallelsDomObjPtr pdom,
} else if (VIR_STORAGE_TYPE_BLOCK) {
virCommandAddArg(cmd, --device);
} else {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Invalid disk type: %d), type);
goto cleanup;
}
@@ -1676,7 +1677,7 @@ static int parallelsAddHdd(parallelsDomObjPtr pdom,
virCommandAddArg(cmd, src);
if (!(strbus = parallelsGetDiskBusName(disk-bus))) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Invalid disk bus: %d), disk-bus);
goto cleanup;
}
@@ -1732,7 +1733,7 @@ parallelsApplyDisksParams(parallelsDomObjPtr pdom,
if (!newdisk) {
if (parallelsRemoveHdd(pdom, olddisk)) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Can't remove disk '%s'
in the specified config),
olddisks[i]-serial);
return -1;
@@ -1757,7 +1758,7 @@ parallelsApplyDisksParams(parallelsDomObjPtr pdom,
snprintf(strpos, 15, %d, newdisk-info.addr.drive.target);
if (!(strbus = parallelsGetDiskBusName(newdisk-bus))) {
-virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED,
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_(Unsupported disk bus: %d), newdisk-bus);
return -1;
}
@@ -1805,58 +1806,58 @@ static int
[libvirt] [PATCH v2 0/5] Parallels: patchset
Another patchset for Parallels libvirt driver. This patchset includes some minor bugfixing and adds some functions required to start OpenStack Changes in v2: fixed wrong error codes in config error handling. Alexander Burluka (5): Parallels: add virNodeGetCPUMap(). Parallels: fix error with video card RAM dimension Parallels: Change config report errors code. Parallels: Add domainCreateWithFlags() function. Parallels: add events emiting while creating domain. src/parallels/parallels_driver.c | 155 --- 1 file changed, 113 insertions(+), 42 deletions(-) -- 1.9.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/5] Parallels: Add domainCreateWithFlags() function.
domainCreateWithFlags function is used by OpenStack Nova to boot
instance.
---
src/parallels/parallels_driver.c | 50
1 file changed, 50 insertions(+)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 0373830..d0f49b9 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -2428,6 +2428,55 @@ parallelsNodeGetCPUMap(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
+static int
+parallelsDomainCreateWithFlags(virDomainPtr domain, unsigned int flags)
+{
+parallelsConnPtr privconn = domain-conn-privateData;
+virDomainObjPtr privdom = NULL;
+int ret = -1;
+
+virCheckFlags(VIR_DOMAIN_START_PAUSED |
+ VIR_DOMAIN_START_AUTODESTROY |
+ VIR_DOMAIN_START_BYPASS_CACHE |
+ VIR_DOMAIN_START_FORCE_BOOT, -1);
+
+parallelsDriverLock(privconn);
+privdom = virDomainObjListFindByUUID(privconn-domains, domain-uuid);
+parallelsDriverUnlock(privconn);
+
+if (!privdom) {
+virReportError(VIR_ERR_NO_DOMAIN, %s,
+ _(no domain with matching uuid));
+ret = -1;
+goto cleanup;
+}
+
+if (virDomainObjIsActive(privdom)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ %s, _(domain is already running));
+ret = -1;
+goto cleanup;
+}
+
+ret = parallelsStart(privdom);
+if (ret == 0) {
+virDomainObjSetState(privdom, VIR_DOMAIN_RUNNING,
+ VIR_DOMAIN_RUNNING_BOOTED);
+} else {
+virDomainObjSetState(privdom, VIR_DOMAIN_CRASHED,
+ VIR_DOMAIN_CRASHED_UNKNOWN);
+}
+
+ret = 0;
+
+ cleanup:
+if (privdom)
+virObjectUnlock(privdom);
+
+return ret;
+}
+
+
static virDriver parallelsDriver = {
.no = VIR_DRV_PARALLELS,
.name = Parallels,
@@ -2459,6 +2508,7 @@ static virDriver parallelsDriver = {
.domainShutdown = parallelsDomainShutdown, /* 0.10.0 */
.domainCreate = parallelsDomainCreate,/* 0.10.0 */
.domainDefineXML = parallelsDomainDefineXML, /* 0.10.0 */
+.domainCreateWithFlags = parallelsDomainCreateWithFlags, /* 1.2.7 */
.nodeGetCPUMap = parallelsNodeGetCPUMap, /* 1.2.6 */
.connectIsEncrypted = parallelsConnectIsEncrypted, /* 1.2.5 */
.connectIsSecure = parallelsConnectIsSecure, /* 1.2.5 */
--
1.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/5] Parallels: fix error with video card RAM dimension
Libvirt measures vram in Kbytes, not in bytes, so calculation
of Mbytes was incorrect. PCS server can take vram argument
with units, so I added K postfix to make params a little bit clearer.
---
src/parallels/parallels_driver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 6534fdb..3806e5e 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -1628,13 +1628,13 @@ parallelsApplyVideoParams(parallelsDomObjPtr pdom,
}
if (old-vram != new-vram) {
-if (new-vram % (1 20) != 0) {
+if (new-vram % (1 10) != 0) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, %s,
_(Video RAM size should be multiple of 1Mb.));
return -1;
}
-snprintf(str_vram, 31, %d, new-vram 20);
+snprintf(str_vram, 31, %dK, new-vram);
str_vram[31] = '\0';
if (parallelsCmdRun(PRLCTL, set, pdom-uuid,
--
1.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/5] Parallels: add virNodeGetCPUMap().
That function caused errors in libvirtd logs when OpenStack Nova
starts VM instance.
---
src/parallels/parallels_driver.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index bb9538f..6534fdb 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -2417,6 +2417,16 @@ parallelsDomainGetVcpus(virDomainPtr domain,
}
+static int
+parallelsNodeGetCPUMap(virConnectPtr conn ATTRIBUTE_UNUSED,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
+{
+return nodeGetCPUMap(cpumap, online, flags);
+}
+
+
static virDriver parallelsDriver = {
.no = VIR_DRV_PARALLELS,
.name = Parallels,
@@ -2448,6 +2458,7 @@ static virDriver parallelsDriver = {
.domainShutdown = parallelsDomainShutdown, /* 0.10.0 */
.domainCreate = parallelsDomainCreate,/* 0.10.0 */
.domainDefineXML = parallelsDomainDefineXML, /* 0.10.0 */
+.nodeGetCPUMap = parallelsNodeGetCPUMap, /* 1.2.6 */
.connectIsEncrypted = parallelsConnectIsEncrypted, /* 1.2.5 */
.connectIsSecure = parallelsConnectIsSecure, /* 1.2.5 */
.connectIsAlive = parallelsConnectIsAlive, /* 1.2.5 */
--
1.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 5/5] Parallels: add events emiting while creating domain.
---
src/parallels/parallels_driver.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index d0f49b9..6a64ef4 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -2433,6 +2433,7 @@ parallelsDomainCreateWithFlags(virDomainPtr domain,
unsigned int flags)
{
parallelsConnPtr privconn = domain-conn-privateData;
virDomainObjPtr privdom = NULL;
+virObjectEventPtr event = NULL;
int ret = -1;
virCheckFlags(VIR_DOMAIN_START_PAUSED |
@@ -2460,9 +2461,15 @@ parallelsDomainCreateWithFlags(virDomainPtr domain,
unsigned int flags)
ret = parallelsStart(privdom);
if (ret == 0) {
+event = virDomainEventLifecycleNewFromObj(privdom,
+ VIR_DOMAIN_EVENT_STARTED,
+ VIR_DOMAIN_EVENT_STARTED_BOOTED);
virDomainObjSetState(privdom, VIR_DOMAIN_RUNNING,
VIR_DOMAIN_RUNNING_BOOTED);
} else {
+event = virDomainEventLifecycleNewFromObj(privdom,
+ VIR_DOMAIN_EVENT_CRASHED,
+ VIR_DOMAIN_EVENT_CRASHED_PANICKED);
virDomainObjSetState(privdom, VIR_DOMAIN_CRASHED,
VIR_DOMAIN_CRASHED_UNKNOWN);
}
@@ -2472,6 +2479,8 @@ parallelsDomainCreateWithFlags(virDomainPtr domain,
unsigned int flags)
cleanup:
if (privdom)
virObjectUnlock(privdom);
+if (event)
+virObjectEventStateQueue(privconn-domainEventState, event);
return ret;
}
--
1.9.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 1/3] conf: Extend loader/ and introduce nvram/
Up to now, users can configure BIOS via the loader/ element. With the upcoming implementation of UEFI this is not enough as BIOS and UEFI are conceptually different. For instance, while BIOS is ROM, UEFI is programmable flash (although all writes to code section are denied). Therefore we need new attribute @type which will differentiate the two. Then, new attribute @readonly is introduced to reflect the fact that some images are RO. Moreover, the OVMF (which is going to be used mostly), works in two modes: 1) Code and UEFI variable store is mixed in one file. 2) Code and UEFI variable store is separated in two files The latter has advantage of updating the UEFI code without losing the configuration. However, in order to represent the latter case we need yet another XML element: nvram/. Currently, it has no additional attributes, it's just a bare element containing path to the variable store file. Signed-off-by: Michal Privoznik mpriv...@redhat.com Acked-by: Laszlo Ersek ler...@redhat.com --- docs/formatdomain.html.in | 19 - docs/schemas/domaincommon.rng | 21 ++ src/conf/domain_conf.c | 87 +- src/conf/domain_conf.h | 22 +- src/libvirt_private.syms | 3 + src/qemu/qemu_command.c| 5 +- src/security/virt-aa-helper.c | 4 +- src/vbox/vbox_common.c | 7 +- src/xenapi/xenapi_driver.c | 3 +- src/xenconfig/xen_common.c | 7 +- src/xenconfig/xen_sxpr.c | 16 ++-- tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml | 40 ++ .../qemuxml2xmlout-pci-bridge-many-disks.xml | 2 +- tests/qemuxml2xmltest.c| 2 + tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml | 2 +- .../sexpr2xml-fv-serial-dev-2-ports.xml| 2 +- .../sexpr2xml-fv-serial-dev-2nd-port.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml | 2 +- .../sexpr2xml-fv-serial-tcp-telnet.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-sound.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-v2.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml | 2 +- tests/xmconfigdata/test-escape-paths.xml | 2 +- tests/xmconfigdata/test-fullvirt-force-hpet.xml| 2 +- tests/xmconfigdata/test-fullvirt-force-nohpet.xml | 2 +- tests/xmconfigdata/test-fullvirt-localtime.xml | 2 +- tests/xmconfigdata/test-fullvirt-net-ioemu.xml | 2 +- tests/xmconfigdata/test-fullvirt-net-netfront.xml | 2 +- tests/xmconfigdata/test-fullvirt-new-cdrom.xml | 2 +- tests/xmconfigdata/test-fullvirt-old-cdrom.xml | 2 +- tests/xmconfigdata/test-fullvirt-parallel-tcp.xml | 2 +- .../test-fullvirt-serial-dev-2-ports.xml | 2 +- .../test-fullvirt-serial-dev-2nd-port.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-file.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-null.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-pipe.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-pty.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-stdio.xml | 2 +- .../test-fullvirt-serial-tcp-telnet.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-tcp.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-udp.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-unix.xml | 2 +- tests/xmconfigdata/test-fullvirt-sound.xml | 2 +- tests/xmconfigdata/test-fullvirt-usbmouse.xml | 2 +- tests/xmconfigdata/test-fullvirt-usbtablet.xml
[libvirt] [PATCH v5 0/3] OVMF exposure
Diff to v4: -rebased onto current master -introduced nvram/@template as requested by Dan -yet another Laszlo's review worked in Michal Privoznik (3): conf: Extend loader/ and introduce nvram/ qemu: Implement extended loader and nvram qemu: Automatically create NVRAM store docs/formatdomain.html.in | 22 +++- docs/schemas/domaincommon.rng | 28 + libvirt.spec.in| 2 + src/Makefile.am| 1 + src/conf/domain_conf.c | 96 ++- src/conf/domain_conf.h | 23 +++- src/libvirt_private.syms | 3 + src/qemu/libvirtd_qemu.aug | 3 + src/qemu/qemu.conf | 14 +++ src/qemu/qemu_command.c| 97 ++- src/qemu/qemu_conf.c | 94 ++ src/qemu/qemu_conf.h | 5 + src/qemu/qemu_process.c| 137 + src/qemu/test_libvirtd_qemu.aug.in | 3 + src/security/security_dac.c| 8 ++ src/security/security_selinux.c| 8 ++ src/security/virt-aa-helper.c | 4 +- src/vbox/vbox_common.c | 7 +- src/xenapi/xenapi_driver.c | 3 +- src/xenconfig/xen_common.c | 7 +- src/xenconfig/xen_sxpr.c | 16 +-- tests/domainschemadata/domain-bios-nvram-empty.xml | 40 ++ .../qemuxml2argvdata/qemuxml2argv-bios-nvram.args | 10 ++ tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.xml | 40 ++ tests/qemuxml2argvtest.c | 2 + .../qemuxml2xmlout-pci-bridge-many-disks.xml | 2 +- tests/qemuxml2xmltest.c| 2 + tests/sexpr2xmldata/sexpr2xml-fv-autoport.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-empty-kernel.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-force-hpet.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-force-nohpet.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-kernel.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-legacy-vfb.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-localtime.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-net-ioemu.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-net-netfront.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-parallel-tcp.xml | 2 +- .../sexpr2xml-fv-serial-dev-2-ports.xml| 2 +- .../sexpr2xml-fv-serial-dev-2nd-port.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-file.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-null.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pipe.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-pty.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-stdio.xml | 2 +- .../sexpr2xml-fv-serial-tcp-telnet.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-tcp.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-udp.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv-serial-unix.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-sound-all.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-sound.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-usbmouse.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-usbtablet.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-utc.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-fv-v2.xml| 2 +- tests/sexpr2xmldata/sexpr2xml-fv.xml | 2 +- tests/sexpr2xmldata/sexpr2xml-no-source-cdrom.xml | 2 +- tests/xmconfigdata/test-escape-paths.xml | 2 +- tests/xmconfigdata/test-fullvirt-force-hpet.xml| 2 +- tests/xmconfigdata/test-fullvirt-force-nohpet.xml | 2 +- tests/xmconfigdata/test-fullvirt-localtime.xml | 2 +- tests/xmconfigdata/test-fullvirt-net-ioemu.xml | 2 +- tests/xmconfigdata/test-fullvirt-net-netfront.xml | 2 +- tests/xmconfigdata/test-fullvirt-new-cdrom.xml | 2 +- tests/xmconfigdata/test-fullvirt-old-cdrom.xml | 2 +- tests/xmconfigdata/test-fullvirt-parallel-tcp.xml | 2 +- .../test-fullvirt-serial-dev-2-ports.xml | 2 +- .../test-fullvirt-serial-dev-2nd-port.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-file.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-null.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-pipe.xml | 2 +- tests/xmconfigdata/test-fullvirt-serial-pty.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-stdio.xml | 2 +- .../test-fullvirt-serial-tcp-telnet.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-tcp.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-udp.xml| 2 +- tests/xmconfigdata/test-fullvirt-serial-unix.xml | 2 +-
[libvirt] [PATCH v5 2/3] qemu: Implement extended loader and nvram
QEMU now supports UEFI with the following command line:
-drive
file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
-drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \
where the first line reflects loader and the second one nvram.
Moreover, these two lines obsoletes the -bios argument.
Note that UEFI is unusable without ACPI. This is handled properly now.
Among with this extension, the variable file is expected to be
writable and hence we need security drivers to label it.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
Acked-by: Laszlo Ersek ler...@redhat.com
---
src/qemu/qemu_command.c| 94 +-
src/security/security_dac.c| 8 ++
src/security/security_selinux.c| 8 ++
.../qemuxml2argvdata/qemuxml2argv-bios-nvram.args | 10 +++
tests/qemuxml2argvtest.c | 2 +
5 files changed, 118 insertions(+), 4 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-bios-nvram.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 2da4c76..886a263 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7299,6 +7299,94 @@ qemuBuildChrDeviceCommandLine(virCommandPtr cmd,
return 0;
}
+static int
+qemuBuilDomainLoaderCommandLine(virCommandPtr cmd,
+virDomainDefPtr def,
+virQEMUCapsPtr qemuCaps)
+{
+int ret = -1;
+virDomainLoaderDefPtr loader = def-os.loader;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int unit = 0;
+
+if (!loader)
+return 0;
+
+switch ((virDomainLoader) loader-type) {
+case VIR_DOMAIN_LOADER_TYPE_ROM:
+virCommandAddArg(cmd, -bios);
+virCommandAddArg(cmd, loader-path);
+break;
+
+case VIR_DOMAIN_LOADER_TYPE_PFLASH:
+/* UEFI is supported only for x86_64 currently */
+if (def-os.arch != VIR_ARCH_X86_64) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(pflash is not supported for %s guest
achitecture),
+ virArchToString(def-os.arch));
+goto cleanup;
+}
+
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this qemu doesn't support -drive));
+goto cleanup;
+}
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_FORMAT)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this qemu doesn't support passing
+ drive format));
+goto cleanup;
+}
+if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NO_ACPI)
+def-features[VIR_DOMAIN_FEATURE_ACPI] != VIR_TRISTATE_SWITCH_ON) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(ACPI must be enabled in order to use UEFI));
+goto cleanup;
+}
+
+virBufferAsprintf(buf,
+ file=%s,if=pflash,format=raw,unit=%d,
+ loader-path, unit);
+unit++;
+
+if (loader-readonly) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this qemu doesn't support passing
+ readonly attribute));
+goto cleanup;
+}
+
+virBufferAsprintf(buf, ,readonly=%s,
+ virTristateSwitchTypeToString(loader-readonly));
+}
+
+virCommandAddArg(cmd, -drive);
+virCommandAddArgBuffer(cmd, buf);
+
+if (loader-nvram) {
+virBufferFreeAndReset(buf);
+virBufferAsprintf(buf,
+ file=%s,if=pflash,format=raw,unit=%d,
+ loader-nvram, unit);
+
+virCommandAddArg(cmd, -drive);
+virCommandAddArgBuffer(cmd, buf);
+}
+break;
+
+case VIR_DOMAIN_LOADER_TYPE_LAST:
+/* nada */
+break;
+}
+
+ret = 0;
+ cleanup:
+virBufferFreeAndReset(buf);
+return ret;
+}
+
qemuBuildCommandLineCallbacks buildCommandLineCallbacks = {
.qemuGetSCSIDeviceSgName = virSCSIDeviceGetSgName,
};
@@ -7454,10 +7542,8 @@ qemuBuildCommandLine(virConnectPtr conn,
virCommandAddArg(cmd, -enable-nesting);
}
-if (def-os.loader) {
-virCommandAddArg(cmd, -bios);
-virCommandAddArg(cmd, def-os.loader-path);
-}
+if (qemuBuilDomainLoaderCommandLine(cmd, def, qemuCaps) 0)
+goto error;
/* Set '-m MB' based on maxmem, because the lower 'memory' limit
* is set post-startup using the balloon driver. If balloon driver
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index e62828e..e398d2c 100644
[libvirt] [PATCH v5 3/3] qemu: Automatically create NVRAM store
When using split UEFI image, it may come handy if libvirt manages per
domain _VARS file automatically. While the _CODE file is RO and can be
shared among multiple domains, you certainly don't want to do that on
the _VARS file. This latter one needs to be per domain. So at the
domain startup process, if it's determined that domain needs _VARS
file it's copied from this master _VARS file. The location of the
master file is configurable in qemu.conf.
Temporary, on per domain basis the location of master NVRAM file can
be overridden by this @template attribute I'm inventing to the
nvram/ element. All it does is holding path to the master NVRAM file
from which local copy is created. If that's the case, the map in
qemu.conf is not consulted.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
docs/formatdomain.html.in | 11 +-
docs/schemas/domaincommon.rng | 9 +-
libvirt.spec.in| 2 +
src/Makefile.am| 1 +
src/conf/domain_conf.c | 11 +-
src/conf/domain_conf.h | 1 +
src/qemu/libvirtd_qemu.aug | 3 +
src/qemu/qemu.conf | 14 +++
src/qemu/qemu_conf.c | 94 ++
src/qemu/qemu_conf.h | 5 +
src/qemu/qemu_process.c| 137 +
src/qemu/test_libvirtd_qemu.aug.in | 3 +
tests/domainschemadata/domain-bios-nvram-empty.xml | 40 ++
13 files changed, 325 insertions(+), 6 deletions(-)
create mode 100644 tests/domainschemadata/domain-bios-nvram-empty.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 589ca0a..fd1e504 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -103,7 +103,7 @@
lt;osgt;
lt;typegt;hvmlt;/typegt;
lt;loader readonly='on'
type='rom'gt;/usr/lib/xen/boot/hvmloaderlt;/loadergt;
-lt;nvramgt;/var/lib/libvirt/nvram/guest_VARS.fdlt;/nvramgt;
+lt;nvram
template='/usr/share/OVMF/OVMF_VARS.fd'gt;/var/lib/libvirt/nvram/guest_VARS.fdlt;/nvramgt;
lt;boot dev='hd'/gt;
lt;boot dev='cdrom'/gt;
lt;bootmenu enable='yes'/gt;
@@ -142,9 +142,12 @@
codepflash/code./dd
dtcodenvram/code/dt
ddSome UEFI firmwares may want to use a non-volatile memory to store
-some variables. In the host, this is represented as a file and the
-path to the file is stored in this element. span class=sinceSince
-1.2.8/span/dd
+some variables. In the host, this is represented as a file and the path
+to the file is stored in this element. Moreover, when the domain is
+started up libvirt copies so called master NVRAM store file defined
+in codeqemu.conf/code. If needed, the codetemplate/code
+attribute can be used to per domain override map of master NVRAM stores
+from the config file. span class=sinceSince 1.2.8/span/dd
dtcodeboot/code/dt
ddThe codedev/code attribute takes one of the values fd, hd,
cdrom or network and is used to specify the next boot device
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 69fce46..195b8c4 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -263,7 +263,14 @@
/optional
optional
element name=nvram
-ref name=absFilePath/
+optional
+ attribute name=template
+ref name=absFilePath/
+ /attribute
+/optional
+optional
+ ref name=absFilePath/
+/optional
/element
/optional
optional
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 3932313..5003750 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1963,6 +1963,7 @@ exit 0
%dir %attr(0750, %{qemu_user}, %{qemu_group})
%{_localstatedir}/lib/libvirt/qemu/
%dir %attr(0750, %{qemu_user}, %{qemu_group})
%{_localstatedir}/lib/libvirt/qemu/channel/
%dir %attr(0750, %{qemu_user}, %{qemu_group})
%{_localstatedir}/lib/libvirt/qemu/channel/target/
+%dir %attr(0750, %{qemu_user}, %{qemu_group})
%{_localstatedir}/lib/libvirt/qemu/nvram/
%dir %attr(0750, %{qemu_user}, %{qemu_group})
%{_localstatedir}/cache/libvirt/qemu/
%{_datadir}/augeas/lenses/libvirtd_qemu.aug
%{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug
@@ -2065,6 +2066,7 @@ exit 0
%dir %attr(0750, %{qemu_user}, %{qemu_group})
%{_localstatedir}/lib/libvirt/qemu/
%dir %attr(0750, %{qemu_user}, %{qemu_group})
%{_localstatedir}/lib/libvirt/qemu/channel/
%dir %attr(0750, %{qemu_user}, %{qemu_group})
%{_localstatedir}/lib/libvirt/qemu/channel/target/
+%dir %attr(0750, %{qemu_user}, %{qemu_group})
%{_localstatedir}/lib/libvirt/qemu/nvram/
%dir %attr(0750, %{qemu_user}, %{qemu_group})
Re: [libvirt] [PATCH v5 3/3] qemu: Automatically create NVRAM store
one question
On 08/22/14 14:08, Michal Privoznik wrote:
When using split UEFI image, it may come handy if libvirt manages per
domain _VARS file automatically. While the _CODE file is RO and can be
shared among multiple domains, you certainly don't want to do that on
the _VARS file. This latter one needs to be per domain. So at the
domain startup process, if it's determined that domain needs _VARS
file it's copied from this master _VARS file. The location of the
master file is configurable in qemu.conf.
Temporary, on per domain basis the location of master NVRAM file can
be overridden by this @template attribute I'm inventing to the
nvram/ element. All it does is holding path to the master NVRAM file
from which local copy is created. If that's the case, the map in
qemu.conf is not consulted.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
docs/formatdomain.html.in | 11 +-
docs/schemas/domaincommon.rng | 9 +-
libvirt.spec.in| 2 +
src/Makefile.am| 1 +
src/conf/domain_conf.c | 11 +-
src/conf/domain_conf.h | 1 +
src/qemu/libvirtd_qemu.aug | 3 +
src/qemu/qemu.conf | 14 +++
src/qemu/qemu_conf.c | 94 ++
src/qemu/qemu_conf.h | 5 +
src/qemu/qemu_process.c| 137
+
src/qemu/test_libvirtd_qemu.aug.in | 3 +
tests/domainschemadata/domain-bios-nvram-empty.xml | 40 ++
13 files changed, 325 insertions(+), 6 deletions(-)
create mode 100644 tests/domainschemadata/domain-bios-nvram-empty.xml
@@ -17682,7 +17684,14 @@ virDomainLoaderDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, type='%s', type);
virBufferEscapeString(buf, %s/loader\n, loader-path);
-virBufferEscapeString(buf, nvram%s/nvram\n, loader-nvram);
+if (loader-nvram || loader-templt) {
+virBufferAddLit(buf, nvram);
+virBufferEscapeString(buf, template='%s', loader-templt);
Shouldn't you protect this virBufferEscapeString() call with an if too?
+if (loader-nvram)
+virBufferEscapeString(buf, %s/nvram\n, loader-nvram);
+else
+virBufferAddLit(buf, /\n);
+}
}
Looks good otherwise.
Thanks
Laszlo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 3/3] qemu: Automatically create NVRAM store
On 22.08.2014 14:26, Laszlo Ersek wrote:
one question
On 08/22/14 14:08, Michal Privoznik wrote:
When using split UEFI image, it may come handy if libvirt manages per
domain _VARS file automatically. While the _CODE file is RO and can be
shared among multiple domains, you certainly don't want to do that on
the _VARS file. This latter one needs to be per domain. So at the
domain startup process, if it's determined that domain needs _VARS
file it's copied from this master _VARS file. The location of the
master file is configurable in qemu.conf.
Temporary, on per domain basis the location of master NVRAM file can
be overridden by this @template attribute I'm inventing to the
nvram/ element. All it does is holding path to the master NVRAM file
from which local copy is created. If that's the case, the map in
qemu.conf is not consulted.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
docs/formatdomain.html.in | 11 +-
docs/schemas/domaincommon.rng | 9 +-
libvirt.spec.in| 2 +
src/Makefile.am| 1 +
src/conf/domain_conf.c | 11 +-
src/conf/domain_conf.h | 1 +
src/qemu/libvirtd_qemu.aug | 3 +
src/qemu/qemu.conf | 14 +++
src/qemu/qemu_conf.c | 94 ++
src/qemu/qemu_conf.h | 5 +
src/qemu/qemu_process.c| 137 +
src/qemu/test_libvirtd_qemu.aug.in | 3 +
tests/domainschemadata/domain-bios-nvram-empty.xml | 40 ++
13 files changed, 325 insertions(+), 6 deletions(-)
create mode 100644 tests/domainschemadata/domain-bios-nvram-empty.xml
@@ -17682,7 +17684,14 @@ virDomainLoaderDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, type='%s', type);
virBufferEscapeString(buf, %s/loader\n, loader-path);
-virBufferEscapeString(buf, nvram%s/nvram\n, loader-nvram);
+if (loader-nvram || loader-templt) {
+virBufferAddLit(buf, nvram);
+virBufferEscapeString(buf, template='%s', loader-templt);
Shouldn't you protect this virBufferEscapeString() call with an if too?
No, virBufferEscapeString() works slightly different to other virBuffer*
APIs. If any of its 3 arguments is NULL is basically NOP.
So if loader-templt (sigh, I couldn't use template as it's c++ keyword,
but that's different story) is NULL (= it wasn't specified in XML) this
line turns into NOP and doesn't output anything.
+if (loader-nvram)
+virBufferEscapeString(buf, %s/nvram\n, loader-nvram);
+else
+virBufferAddLit(buf, /\n);
+}
You were confused by this, weren't you? This is basically to distinguish
cases if nvram path was specified or not. If it was, we need to print
out nvram/some/path/nvram otherwise the partially written nvram
element needs to be enclosed.
}
Looks good otherwise.
Cool!
Thanks
Laszlo
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/5] Parallels: Change config report errors code.
On 08/22/2014 01:04 PM, Alexander Burluka wrote: Wrong error code in config errors reporting was used. Fixed it. --- src/parallels/parallels_driver.c | 81 1 file changed, 41 insertions(+), 40 deletions(-) ACK and pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/5] Parallels: add virNodeGetCPUMap().
On 08/22/2014 01:04 PM, Alexander Burluka wrote:
That function caused errors in libvirtd logs when OpenStack Nova
starts VM instance.
---
src/parallels/parallels_driver.c | 11 +++
1 file changed, 11 insertions(+)
diff --git a/src/parallels/parallels_driver.c
b/src/parallels/parallels_driver.c
index bb9538f..6534fdb 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -2417,6 +2417,16 @@ parallelsDomainGetVcpus(virDomainPtr domain,
}
+static int
+parallelsNodeGetCPUMap(virConnectPtr conn ATTRIBUTE_UNUSED,
+ unsigned char **cpumap,
+ unsigned int *online,
+ unsigned int flags)
+{
+return nodeGetCPUMap(cpumap, online, flags);
+}
+
+
static virDriver parallelsDriver = {
.no = VIR_DRV_PARALLELS,
.name = Parallels,
@@ -2448,6 +2458,7 @@ static virDriver parallelsDriver = {
.domainShutdown = parallelsDomainShutdown, /* 0.10.0 */
.domainCreate = parallelsDomainCreate,/* 0.10.0 */
.domainDefineXML = parallelsDomainDefineXML, /* 0.10.0 */
+.nodeGetCPUMap = parallelsNodeGetCPUMap, /* 1.2.6 */
s/1.2.6/1.2.8/
ACK and pushed.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 2/5] Parallels: fix error with video card RAM dimension
On 08/22/2014 01:04 PM, Alexander Burluka wrote: Libvirt measures vram in Kbytes, not in bytes, so calculation of Mbytes was incorrect. PCS server can take vram argument with units, so I added K postfix to make params a little bit clearer. --- src/parallels/parallels_driver.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) ACK and pushed. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 4/5] Parallels: Add domainCreateWithFlags() function.
On 08/22/2014 01:04 PM, Alexander Burluka wrote:
domainCreateWithFlags function is used by OpenStack Nova to boot
instance.
---
src/parallels/parallels_driver.c | 50
1 file changed, 50 insertions(+)
diff --git a/src/parallels/parallels_driver.c
b/src/parallels/parallels_driver.c
index 0373830..d0f49b9 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -2428,6 +2428,55 @@ parallelsNodeGetCPUMap(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
+static int
+parallelsDomainCreateWithFlags(virDomainPtr domain, unsigned int flags)
+{
+parallelsConnPtr privconn = domain-conn-privateData;
+virDomainObjPtr privdom = NULL;
+int ret = -1;
+
+virCheckFlags(VIR_DOMAIN_START_PAUSED |
+ VIR_DOMAIN_START_AUTODESTROY |
+ VIR_DOMAIN_START_BYPASS_CACHE |
+ VIR_DOMAIN_START_FORCE_BOOT, -1);
You don't seem to be using the flags anywhere. I don't think we should pretend
they are supported if they have no effect.
Also, shouldn't this share more code with parallelsDomainCreate?
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 3/3] qemu: Automatically create NVRAM store
On 08/22/14 14:43, Michal Privoznik wrote:
On 22.08.2014 14:26, Laszlo Ersek wrote:
one question
On 08/22/14 14:08, Michal Privoznik wrote:
When using split UEFI image, it may come handy if libvirt manages per
domain _VARS file automatically. While the _CODE file is RO and can be
shared among multiple domains, you certainly don't want to do that on
the _VARS file. This latter one needs to be per domain. So at the
domain startup process, if it's determined that domain needs _VARS
file it's copied from this master _VARS file. The location of the
master file is configurable in qemu.conf.
Temporary, on per domain basis the location of master NVRAM file can
be overridden by this @template attribute I'm inventing to the
nvram/ element. All it does is holding path to the master NVRAM file
from which local copy is created. If that's the case, the map in
qemu.conf is not consulted.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
docs/formatdomain.html.in | 11 +-
docs/schemas/domaincommon.rng | 9 +-
libvirt.spec.in| 2 +
src/Makefile.am| 1 +
src/conf/domain_conf.c | 11 +-
src/conf/domain_conf.h | 1 +
src/qemu/libvirtd_qemu.aug | 3 +
src/qemu/qemu.conf | 14 +++
src/qemu/qemu_conf.c | 94
++
src/qemu/qemu_conf.h | 5 +
src/qemu/qemu_process.c| 137
+
src/qemu/test_libvirtd_qemu.aug.in | 3 +
tests/domainschemadata/domain-bios-nvram-empty.xml | 40 ++
13 files changed, 325 insertions(+), 6 deletions(-)
create mode 100644 tests/domainschemadata/domain-bios-nvram-empty.xml
@@ -17682,7 +17684,14 @@ virDomainLoaderDefFormat(virBufferPtr buf,
virBufferAsprintf(buf, type='%s', type);
virBufferEscapeString(buf, %s/loader\n, loader-path);
-virBufferEscapeString(buf, nvram%s/nvram\n, loader-nvram);
+if (loader-nvram || loader-templt) {
+virBufferAddLit(buf, nvram);
+virBufferEscapeString(buf, template='%s', loader-templt);
Shouldn't you protect this virBufferEscapeString() call with an if too?
No, virBufferEscapeString() works slightly different to other virBuffer*
APIs. If any of its 3 arguments is NULL is basically NOP.
So if loader-templt (sigh, I couldn't use template as it's c++ keyword,
but that's different story) is NULL (= it wasn't specified in XML) this
line turns into NOP and doesn't output anything.
OK. I thought it'd crash. (And this time I didn't have the cycles to
check the callee myself.)
+if (loader-nvram)
+virBufferEscapeString(buf, %s/nvram\n, loader-nvram);
+else
+virBufferAddLit(buf, /\n);
+}
You were confused by this, weren't you? This is basically to distinguish
cases if nvram path was specified or not. If it was, we need to print
out nvram/some/path/nvram otherwise the partially written nvram
element needs to be enclosed.
Nah, I got that. I was only worried about a NULL deref in the template
formatting.
series
Acked-by: Laszlo Ersek ler...@redhat.com
Hopefully Dan will be OK with this version.
Cheers,
Laszlo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix spacing around commas
On some places in the libvirt code we have:
f(a,z)
instead of
f(a, z)
This trivial patch fixes couple of such occurrences.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
Pushed under trivial rule.
HACKING | 2 +-
docs/hacking.html.in| 2 +-
examples/object-events/event-test.c | 2 +-
src/driver.h| 2 +-
src/internal.h | 26 +-
src/locking/lock_driver_lockd.c | 2 +-
src/locking/lock_driver_sanlock.c | 2 +-
src/lxc/lxc_conf.c | 2 +-
src/nwfilter/nwfilter_dhcpsnoop.c | 2 +-
src/qemu/qemu_conf.c| 2 +-
src/storage/parthelper.c| 2 +-
11 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/HACKING b/HACKING
index 4cf2906..88a4286 100644
--- a/HACKING
+++ b/HACKING
@@ -477,7 +477,7 @@ Use parenthesis when checking if a macro is defined, and
use indentation to
track nesting:
#if defined(HAVE_POSIX_FALLOCATE) !defined(HAVE_FALLOCATE)
- # define fallocate(a,ignored,b,c) posix_fallocate(a,b,c)
+ # define fallocate(a, ignored, b, c) posix_fallocate(a, b, c)
#endif
diff --git a/docs/hacking.html.in b/docs/hacking.html.in
index 6a92f46..bc76542 100644
--- a/docs/hacking.html.in
+++ b/docs/hacking.html.in
@@ -614,7 +614,7 @@
/p
pre
#if defined(HAVE_POSIX_FALLOCATE) amp;amp; !defined(HAVE_FALLOCATE)
- # define fallocate(a,ignored,b,c) posix_fallocate(a,b,c)
+ # define fallocate(a, ignored, b, c) posix_fallocate(a, b, c)
#endif
/pre
diff --git a/examples/object-events/event-test.c
b/examples/object-events/event-test.c
index 2a5a83b..d6cfe46 100644
--- a/examples/object-events/event-test.c
+++ b/examples/object-events/event-test.c
@@ -10,7 +10,7 @@
#include libvirt/virterror.h
#define VIR_DEBUG(fmt) printf(%s:%d: fmt \n, __func__, __LINE__)
-#define STREQ(a,b) (strcmp(a,b) == 0)
+#define STREQ(a, b) (strcmp(a, b) == 0)
#ifndef ATTRIBUTE_UNUSED
# define ATTRIBUTE_UNUSED __attribute__((__unused__))
diff --git a/src/driver.h b/src/driver.h
index 158df79..ba7c1fc 100644
--- a/src/driver.h
+++ b/src/driver.h
@@ -78,7 +78,7 @@ typedef enum {
* != 0 Feature is supported.
* 0 Feature is not supported.
*/
-# define VIR_DRV_SUPPORTS_FEATURE(drv,conn,feature) \
+# define VIR_DRV_SUPPORTS_FEATURE(drv, conn, feature) \
((drv)-connectSupportsFeature ?\
(drv)-connectSupportsFeature((conn), (feature)) 0 : 0)
diff --git a/src/internal.h b/src/internal.h
index d355344..f6a88b2 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -78,16 +78,16 @@
# endif
/* String equality tests, suggested by Jim Meyering. */
-# define STREQ(a,b) (strcmp(a,b) == 0)
-# define STRCASEEQ(a,b) (c_strcasecmp(a,b) == 0)
-# define STRNEQ(a,b) (strcmp(a,b) != 0)
-# define STRCASENEQ(a,b) (c_strcasecmp(a,b) != 0)
-# define STREQLEN(a,b,n) (strncmp(a,b,n) == 0)
-# define STRCASEEQLEN(a,b,n) (c_strncasecmp(a,b,n) == 0)
-# define STRNEQLEN(a,b,n) (strncmp(a,b,n) != 0)
-# define STRCASENEQLEN(a,b,n) (c_strncasecmp(a,b,n) != 0)
-# define STRPREFIX(a,b) (strncmp(a,b,strlen(b)) == 0)
-# define STRSKIP(a,b) (STRPREFIX(a,b) ? (a) + strlen(b) : NULL)
+# define STREQ(a, b) (strcmp(a, b) == 0)
+# define STRCASEEQ(a, b) (c_strcasecmp(a, b) == 0)
+# define STRNEQ(a, b) (strcmp(a, b) != 0)
+# define STRCASENEQ(a, b) (c_strcasecmp(a, b) != 0)
+# define STREQLEN(a, b, n) (strncmp(a, b, n) == 0)
+# define STRCASEEQLEN(a, b, n) (c_strncasecmp(a, b, n) == 0)
+# define STRNEQLEN(a, b, n) (strncmp(a, b, n) != 0)
+# define STRCASENEQLEN(a, b, n) (c_strncasecmp(a, b, n) != 0)
+# define STRPREFIX(a, b) (strncmp(a, b, strlen(b)) == 0)
+# define STRSKIP(a, b) (STRPREFIX(a, b) ? (a) + strlen(b) : NULL)
# define STREQ_NULLABLE(a, b) \
((a) ? (b) STREQ((a) ? (a) : , (b) ? (b) : ) : !(b))
@@ -109,7 +109,7 @@
#define __GNUC_PREREQ(maj, min)\
((__GNUC__ 16) + __GNUC_MINOR__ = ((maj) 16) + (min))
# else
-#define __GNUC_PREREQ(maj,min) 0
+#define __GNUC_PREREQ(maj, min) 0
# endif
/* Work around broken limits.h on debian etch */
@@ -162,10 +162,10 @@
*/
# ifndef ATTRIBUTE_FMT_PRINTF
# if __GNUC_PREREQ (4, 4)
-#define ATTRIBUTE_FMT_PRINTF(fmtpos,argpos) \
+#define ATTRIBUTE_FMT_PRINTF(fmtpos, argpos) \
__attribute__((__format__ (__gnu_printf__, fmtpos, argpos)))
# else
-#define ATTRIBUTE_FMT_PRINTF(fmtpos,argpos) \
+#define ATTRIBUTE_FMT_PRINTF(fmtpos, argpos) \
__attribute__((__format__ (__printf__, fmtpos, argpos)))
# endif
# endif
diff --git a/src/locking/lock_driver_lockd.c b/src/locking/lock_driver_lockd.c
index 367d0ce..0a40e94 100644
--- a/src/locking/lock_driver_lockd.c
+++ b/src/locking/lock_driver_lockd.c
@@ -99,7 +99,7 @@ static int virLockManagerLockDaemonLoadConfig(const char
*configFile)
Re: [libvirt] [PATCH v2 4/5] Parallels: Add domainCreateWithFlags() function.
This check supresses unused variable warning. I thought that it would be
rather natural that just ATTRIBUTE_UNUSED. This flags will be used in
new version of our driver based on our published SDK
(https://github.com/Parallels/parallels-sdk).
You are perfectly right, this code is a quite similiar to
parallelsDomainState. I can rewrite it to make a little bit clearer or
you may reject this patch and wait for a SDK-version. Current version of
that function doesn't mean a lot for us.
Thank you!
On 08/22/2014 04:51 PM, Ján Tomko wrote:
On 08/22/2014 01:04 PM, Alexander Burluka wrote:
domainCreateWithFlags function is used by OpenStack Nova to boot
instance.
---
src/parallels/parallels_driver.c | 50
1 file changed, 50 insertions(+)
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 0373830..d0f49b9 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -2428,6 +2428,55 @@ parallelsNodeGetCPUMap(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
+static int
+parallelsDomainCreateWithFlags(virDomainPtr domain, unsigned int flags)
+{
+parallelsConnPtr privconn = domain-conn-privateData;
+virDomainObjPtr privdom = NULL;
+int ret = -1;
+
+virCheckFlags(VIR_DOMAIN_START_PAUSED |
+ VIR_DOMAIN_START_AUTODESTROY |
+ VIR_DOMAIN_START_BYPASS_CACHE |
+ VIR_DOMAIN_START_FORCE_BOOT, -1);
You don't seem to be using the flags anywhere. I don't think we should pretend
they are supported if they have no effect.
Also, shouldn't this share more code with parallelsDomainCreate?
Jan
--
Regards,
Alexander Burluka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3] numatune: setting --mode does not work well
When trying to set numatune mode directly using virsh numatune command,
correct error is raised, however numatune structure was not deallocated,
thus resulting in creating an empty numatune element in the guest XML,
if none was present before. Running the same command aftewards results
in a successful change with broken XML structure. Patch fixes the
deallocation problem as well as checking for invalid attribute
combination VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO + a nonempty nodeset.
Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1129998
---
src/conf/numatune_conf.c | 45 ++
...-numad-auto-vcpu-static-numatune-no-nodeset.xml | 31 +++
tests/qemuxml2argvtest.c | 1 +
3 files changed, 62 insertions(+), 15 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune-no-nodeset.xml
diff --git a/src/conf/numatune_conf.c b/src/conf/numatune_conf.c
index 48d1d04..ff0f3eb 100644
--- a/src/conf/numatune_conf.c
+++ b/src/conf/numatune_conf.c
@@ -437,14 +437,27 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr,
int mode,
virBitmapPtr nodeset)
{
-bool create = !*numatunePtr; /* Whether we are creating new struct */
+bool created = false;
int ret = -1;
-virDomainNumatunePtr numatune = NULL;
+virDomainNumatunePtr numatune;
/* No need to do anything in this case */
if (mode == -1 placement == -1 !nodeset)
return 0;
+if (!(*numatunePtr)) {
+if (VIR_ALLOC(*numatunePtr) 0)
+goto cleanup;
+
+created = true;
+if (mode == -1)
+mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
+if (placement == -1)
+placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_DEFAULT;
+}
+
+numatune = *numatunePtr;
+
/* Range checks */
if (mode != -1
(mode 0 || mode = VIR_DOMAIN_NUMATUNE_MEM_LAST)) {
@@ -453,6 +466,7 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr,
mode);
goto cleanup;
}
+
if (placement != -1
(placement 0 || placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_LAST)) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
@@ -461,21 +475,9 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr,
goto cleanup;
}
-if (create VIR_ALLOC(*numatunePtr) 0)
-goto cleanup;
-numatune = *numatunePtr;
-
-if (create) {
-/* Defaults for new struct */
-if (mode == -1)
-mode = VIR_DOMAIN_NUMATUNE_MEM_STRICT;
-
-if (placement == -1)
-placement = VIR_DOMAIN_NUMATUNE_PLACEMENT_DEFAULT;
-}
-
if (mode != -1)
numatune-memory.mode = mode;
+
if (nodeset) {
virBitmapFree(numatune-memory.nodeset);
numatune-memory.nodeset = virBitmapNewCopy(nodeset);
@@ -500,13 +502,26 @@ virDomainNumatuneSet(virDomainNumatunePtr *numatunePtr,
goto cleanup;
}
+/* setting nodeset when placement auto is invalid */
+if (placement == VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO
+numatune-memory.nodeset) {
+virBitmapFree(numatune-memory.nodeset);
+numatune-memory.nodeset = NULL;
+}
+
if (placement != -1)
numatune-memory.placement = placement;
numatune-memory.specified = true;
ret = 0;
+
cleanup:
+if (ret 0 created) {
+virDomainNumatuneFree(*numatunePtr);
+*numatunePtr = NULL;
+}
+
return ret;
}
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune-no-nodeset.xml
b/tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune-no-nodeset.xml
new file mode 100644
index 000..b3cb5c5
--- /dev/null
+++
b/tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune-no-nodeset.xml
@@ -0,0 +1,31 @@
+domain type='qemu'
+ nameQEMUGuest1/name
+ uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
+ memory unit='KiB'219136/memory
+ currentMemory unit='KiB'219136/currentMemory
+ vcpu placement='auto'2/vcpu
+ numatune
+memory mode='interleave' placement='static'/
+ /numatune
+ os
+type arch='i686' machine='pc'hvm/type
+boot dev='hd'/
+ /os
+ cpu
+topology sockets='2' cores='1' threads='1'/
+ /cpu
+ clock offset='utc'/
+ on_poweroffdestroy/on_poweroff
+ on_rebootrestart/on_reboot
+ on_crashdestroy/on_crash
+ devices
+emulator/usr/bin/qemu/emulator
+disk type='block' device='disk'
+ source dev='/dev/HostVG/QEMUGuest1'/
+ target dev='hda' bus='ide'/
+ address type='drive' controller='0' bus='0' target='0' unit='0'/
+/disk
+controller type='ide' index='0'/
+memballoon model='virtio'/
+ /devices
+/domain
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 65dc9c7..b289d0e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1223,6 +1223,7 @@ mymain(void)
Re: [libvirt] [PATCH v2 3/3] virstoragefile: Refactor virStorageFileResize
On 08/21/2014 06:51 PM, John Ferlan wrote:
On 08/21/2014 11:53 AM, Ján Tomko wrote:
On 08/11/2014 10:30 PM, John Ferlan wrote:
Currently virStorageFileResize() function uses build conditionals to
choose either the posix_fallocate() or mmap() with no fallback in order
to preallocate the space in the newly resized file.
This patch will modify the logic in order to allow fallbacks in the event
that posix_fallocate() or the sys_fallocate syscall() doesn't work properly.
The fallback will be to use the slow safewrite of zero filled buffers to
the file.
Use the virFileFdPosixFallocate() rather than a local function.
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/util/virstoragefile.c | 52
+++
1 file changed, 34 insertions(+), 18 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 5b6b2f5..4d37de1 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1086,6 +1086,39 @@ virStorageFileChainGetBroken(virStorageSourcePtr
chain,
return 0;
}
+static int
+resizeSysFallocate(const char *path,
+ int fd,
+ off_t offset,
+ off_t len)
+{
+int rc = -1;
+#if HAVE_SYS_SYSCALL_H defined(SYS_fallocate)
+if ((rc = syscall(SYS_fallocate, fd, 0, offset, len)) != 0) {
+virReportSystemError(errno,
+ _(Failed to pre-allocate space for
+ file '%s'), path);
I think this should not log an error, since we have a fallback.
VIR_DEBUG maybe?
Yep - right. Although to match the other path, VIR_WARN...
+}
+#endif
+return rc;
+}
+
+static int
+doResize(const char *path,
+ int fd,
+ off_t offset,
+ off_t len)
+{
+if (virFileFdPosixFallocate(fd, offset, len) == 0 ||
+resizeSysFallocate(path, fd, offset, len) == 0 ||
+safezero(fd, offset, len) == 0)
+return 0;
+
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, %s,
+ _(preallocate is not supported on this platform));
+return -1;
safezero is always supported. And this function should do:
return safezero(fd, offset, len);
Hmm.. Perhaps a better way to do this would be to modify safezero() to
add a 4th boolean parameter resize and make the safezero_mmap() be
the false side and the check/call to SYS_fallocate() be the true side.
That way all the logic resides in virfile.c
How about 'fallback_to_mmap' instead of resize?
Or even simpler, we don't really need a different set of fallbacks for resize
than the other uses. It seems the patch adding the preallocation to FileResize
should have used safezero instead of reimplementing it.
(I also wonder if there are systems without posix_fallocate, but having the
syscall...)
ACK to safezero implementation with all four methods:
int
safezero(int fd, off_t offset, off_t len)
{
if (virFileFdPosixFallocate(fd, offset, len) == 0)
return 0;
if (safezero_sys_fallocate(fd, offset, len) == 0)
return 0;
if (safezero_mmap(fd, offset, len) == 0)
return 0;
return safezero_slow(fd, offset, len);
}
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] parallels: build with parallels SDK
On 21.08.2014 22:36, Dmitry Guryanov wrote: Executing prlctl command is not an optimal way to interact with Parallels Cloud Server (PCS), it's better to use parallels SDK, which is a remote API to paralles dispatcher service. We prepared opensource version of this SDK and published it on github, it's distributed under LGPL license. Here is a git repo: https://github.com/Parallels/parallels-sdk. To build with parallels SDK user should get compiler and linker options from pkg-config 'parallels-sdk' file. So fix checks in configure script and build with parallels SDK, if that pkg-config file exists and add gcc options to makefile. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- configure.ac| 11 +-- src/Makefile.am | 4 +++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index f93c6c2..4318d24 100644 --- a/configure.ac +++ b/configure.ac @@ -1046,15 +1046,14 @@ dnl dnl Checks for the Parallels driver dnl +PKG_CHECK_MODULES([PARALLELS_SDK], [parallels-sdk], [PARALLELS_SDK_FOUND=yes], [PARALLELS_SDK_FOUND=no]) + if test $with_parallels = check; then -with_parallels=$with_linux -if test ! $host_cpu = 'x86_64'; then -with_parallels=no -fi +with_parallels=$PARALLELS_SDK_FOUND fi -if test $with_parallels = yes test $with_linux = no; then -AC_MSG_ERROR([The Parallels driver can be enabled on Linux only.]) +if test $with_parallels = yes test $PARALLELS_SDK_FOUND = no; then +AC_MSG_ERROR([Parallels Virtualization SDK is needed to build the Parallels driver.]) fi if test $with_parallels = yes; then diff --git a/src/Makefile.am b/src/Makefile.am index 538530e..dad7c7f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1352,7 +1352,9 @@ if WITH_PARALLELS noinst_LTLIBRARIES += libvirt_driver_parallels.la libvirt_la_BUILT_LIBADD += libvirt_driver_parallels.la libvirt_driver_parallels_la_CFLAGS = \ - -I$(top_srcdir)/src/conf $(AM_CFLAGS) + -I$(top_srcdir)/src/conf $(AM_CFLAGS) \ + $(PARALLELS_SDK_CFLAGS) +libvirt_driver_parallels_la_LIBADD = $(PARALLELS_SDK_LIBS) libvirt_driver_parallels_la_SOURCES = $(PARALLELS_DRIVER_SOURCES) endif WITH_PARALLELS So previously, this was enabled for x86_64 linux hosts, now you need to have SDK. I understand why you are doing that but perhaps do we want to enable backward compatibility and let users use prlctl command? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] parallels: login to parallels SDK
On 21.08.2014 22:36, Dmitry Guryanov wrote:
Add files parallels_sdk.c and parallels_sdk.h for code
which works with SDK, so libvirt's code will not mix with
dealing with parallels SDK.
To use Parallels SDK you must first call PrlApi_InitEx function,
and then you will be able to connect to a server with
PrlSrv_LoginLocalEx function. When you've done you must call
PrlApi_Deinit. So let's call PrlApi_InitEx on first .connectOpen,
count number of connections and deinitialize, when this counter
becomes zero.
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
po/POTFILES.in | 1 +
src/Makefile.am | 4 +++-
src/parallels/parallels_driver.c | 46 +++-
src/parallels/parallels_utils.h | 3 +++
4 files changed, 52 insertions(+), 2 deletions(-)
This fails 'make syntax-check' for me.
diff --git a/po/POTFILES.in b/po/POTFILES.in
index f17b35f..4c1302d 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -96,6 +96,7 @@ src/openvz/openvz_driver.c
src/openvz/openvz_util.c
src/parallels/parallels_driver.c
src/parallels/parallels_network.c
+src/parallels/parallels_sdk.c
src/parallels/parallels_utils.c
src/parallels/parallels_utils.h
src/parallels/parallels_storage.c
diff --git a/src/Makefile.am b/src/Makefile.am
index dad7c7f..d4c6465 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -787,7 +787,9 @@ PARALLELS_DRIVER_SOURCES =
\
parallels/parallels_utils.c \
parallels/parallels_utils.h \
parallels/parallels_storage.c \
- parallels/parallels_network.c
+ parallels/parallels_network.c \
+ parallels/parallels_sdk.h \
+ parallels/parallels_sdk.c
BHYVE_DRIVER_SOURCES =\
bhyve/bhyve_capabilities.c \
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index bb9538f..7dc9963 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -55,6 +55,7 @@
#include parallels_driver.h
#include parallels_utils.h
+#include parallels_sdk.h
#define VIR_FROM_THIS VIR_FROM_PARALLELS
@@ -73,6 +74,9 @@ VIR_LOG_INIT(parallels.parallels_driver);
#define IS_CT(def) (STREQ_NULLABLE(def-os.type, exe))
+unsigned int numConns = 0;
+virMutex numConnsLock;
1: ^^^
+
static int parallelsConnectClose(virConnectPtr conn);
static const char * parallelsGetDiskBusName(int bus) {
@@ -929,9 +933,25 @@ parallelsOpenDefault(virConnectPtr conn)
if (virMutexInit(privconn-lock) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(cannot initialize mutex));
-goto error;
+goto err_free;
}
+virMutexLock(numConnsLock);
+numConns++;
+
+if (numConns == 1) {
+if (prlsdkInit()) {
+VIR_DEBUG(%s, _(Can't initialize Parallels SDK));
+virMutexUnlock(numConnsLock);
+goto err_free;
+}
+}
+
+virMutexUnlock(numConnsLock);
+
+if (prlsdkConnect(privconn) 0)
+goto err_free;
+
if (!(privconn-caps = parallelsBuildCapabilities()))
goto error;
@@ -953,6 +973,9 @@ parallelsOpenDefault(virConnectPtr conn)
virObjectUnref(privconn-domains);
virObjectUnref(privconn-caps);
virStoragePoolObjListFree(privconn-pools);
+prlsdkDisconnect(privconn);
+prlsdkDeinit();
+ err_free:
VIR_FREE(privconn);
return VIR_DRV_OPEN_ERROR;
}
@@ -999,8 +1022,17 @@ parallelsConnectClose(virConnectPtr conn)
virObjectUnref(privconn-caps);
virObjectUnref(privconn-xmlopt);
virObjectUnref(privconn-domains);
+prlsdkDisconnect(privconn);
conn-privateData = NULL;
+virMutexLock(numConnsLock);
+numConns--;
+
+if (numConns == 0)
+prlsdkDeinit();
+
+virMutexUnlock(numConnsLock);
+
parallelsDriverUnlock(privconn);
virMutexDestroy(privconn-lock);
@@ -2453,6 +2485,12 @@ static virDriver parallelsDriver = {
.connectIsAlive = parallelsConnectIsAlive, /* 1.2.5 */
};
+static virStateDriver parallelsStateDriver = {
+ .name = parallels,
+ .stateInitialize = parallelsStateInitialize,
+ .stateCleanup = parallelsStateCleanup,
+};
+
This is not called (read registered) anywhere so the
state{Initialize,Cleanup} members are not gonna be called at all.
Moreover, if you intent to turn this into state driver, then the [1] are
good candidates to become members of some privateData struct of the
state driver.
/**
* parallelsRegister:
*
@@ -2471,6 +2509,12 @@ parallelsRegister(void)
VIR_FREE(prlctl_path);
+if (virMutexInit(numConnsLock) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(cannot initialize mutex));
Re: [libvirt] [PATCH v2 3/3] virstoragefile: Refactor virStorageFileResize
On 08/22/2014 10:32 AM, Ján Tomko wrote:
On 08/21/2014 06:51 PM, John Ferlan wrote:
...snip...
Hmm.. Perhaps a better way to do this would be to modify safezero() to
add a 4th boolean parameter resize and make the safezero_mmap() be
the false side and the check/call to SYS_fallocate() be the true side.
That way all the logic resides in virfile.c
How about 'fallback_to_mmap' instead of resize?
Or even simpler, we don't really need a different set of fallbacks for resize
than the other uses. It seems the patch adding the preallocation to FileResize
should have used safezero instead of reimplementing it.
(I also wonder if there are systems without posix_fallocate, but having the
syscall...)
ACK to safezero implementation with all four methods:
int
safezero(int fd, off_t offset, off_t len)
{
if (virFileFdPosixFallocate(fd, offset, len) == 0)
return 0;
if (safezero_sys_fallocate(fd, offset, len) == 0)
return 0;
if (safezero_mmap(fd, offset, len) == 0)
return 0;
return safezero_slow(fd, offset, len);
}
OK - I'll do this (although since virFileFdPosixFallocate only needs to
be local now, I'll rename it to safezero_posix_fallocate() and make it
static to the module as well as removing it from libvirt_private.syms).
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 3/3] virstoragefile: Refactor virStorageFileResize
On Fri, Aug 22, 2014 at 04:32:09PM +0200, Ján Tomko wrote:
On 08/21/2014 06:51 PM, John Ferlan wrote:
ACK to safezero implementation with all four methods:
int
safezero(int fd, off_t offset, off_t len)
{
if (virFileFdPosixFallocate(fd, offset, len) == 0)
return 0;
if (safezero_sys_fallocate(fd, offset, len) == 0)
return 0;
if (safezero_mmap(fd, offset, len) == 0)
return 0;
return safezero_slow(fd, offset, len);
}
Huh, why would we want todo that ? safezero() will always use the
posix_fallocate() function if it exists at build time, otherwise it
will be built for mmap, or even write(). If posix_fallocate() is used,
it will try the fast allocation strategy and fallback to manually
filling with zeros if not available without libvirt needing todo the
fallback itself. So we don't need to do any of this dynamically
fallback at runtime - just use safezero() as it exists today.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] virfile: Refactor safezero, introduce virFileFdPosixFallocate
On Mon, Aug 11, 2014 at 04:30:19PM -0400, John Ferlan wrote: Currently the safezero() function uses build conditionals to choose either the posix_fallocate() or mmap() with a fallback to safewrite() in order to preallocate a file. This patch will modify the logic in order to allow fallbacks in the event that posix_fallocate() or the ftruncate()and mmap() doesn't work properly. The fallback will be to use the slow safewrite of zero filled buffers to the file. Have you actually encountered failing of posix_fallocate() in the real world ? It is supposed to automatically fallback to the equivalent of writing zeros if the filesystem / kernel does not support it, so we should not have todo runtime fallback ourselves. The existance of fallback is the main distinction between the posix_fallocate() and fallocate() system calls. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] virfile: Refactor safezero, introduce virFileFdPosixFallocate
On 08/22/2014 10:46 AM, Daniel P. Berrange wrote: On Mon, Aug 11, 2014 at 04:30:19PM -0400, John Ferlan wrote: Currently the safezero() function uses build conditionals to choose either the posix_fallocate() or mmap() with a fallback to safewrite() in order to preallocate a file. This patch will modify the logic in order to allow fallbacks in the event that posix_fallocate() or the ftruncate()and mmap() doesn't work properly. The fallback will be to use the slow safewrite of zero filled buffers to the file. Have you actually encountered failing of posix_fallocate() in the real world ? It is supposed to automatically fallback to the equivalent of writing zeros if the filesystem / kernel does not support it, so we should not have todo runtime fallback ourselves. The existance of fallback is the main distinction between the posix_fallocate() and fallocate() system calls. It wasn't so much as a failure as unexpected results - the key being that the resulting created (or resized) file was not sized as expected. For an NFS target the results are not what was expected. I've left some history in the prior set of patches with the following probably having the most details: http://www.redhat.com/archives/libvir-list/2014-August/msg00367.html As for real world - this bug came as a result of a virt-test test which was going through all the various sizes/options and found this result to be incorrect - at least in the eyes of the tester. John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] iotune: setting an invalid value now reports error
On 08/21/2014 02:28 PM, Erik Skultety wrote:
When trying to set an invalid value into iotune element, standard
behavior was to not report any error, rather to reset all affected
subelements of the iotune element back to 0 which results in ignoring
those particular subelements by XML generator. Patch further
examines the return code of the virXPathULongLong function
and in case of an invalid non-integer value raises an error.
Fixed to preserve consistency with invalid value checking
of other elements.
Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1131811
---
src/conf/domain_conf.c | 55
+++---
1 file changed, 43 insertions(+), 12 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9557020..592aa94 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5416,6 +5416,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
char *mirrorType = NULL;
int expected_secret_usage = -1;
int auth_secret_usage = -1;
+int ret = 0;
if (!(def = virDomainDiskDefNew()))
return NULL;
@@ -5644,40 +5645,70 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt,
goto error;
}
} else if (xmlStrEqual(cur-name, BAD_CAST iotune)) {
-if (virXPathULongLong(string(./iotune/total_bytes_sec),
+ret = virXPathULongLong(string(./iotune/total_bytes_sec),
ctxt,
The indentation is off here - c and should be on the same column.
- def-blkdeviotune.total_bytes_sec)
0) {
+ def-blkdeviotune.total_bytes_sec);
+if (ret 0 ret != -2) {
def-blkdeviotune.total_bytes_sec = 0;
+} else if (ret == -2) {
+virReportError(VIR_ERR_XML_ERROR, %s,
+ _(total throughput limit must be an
integer));
+goto error;
}
I'd rewrite these conditions as
if (ret == -2) {
goto error;
} else if (ret 0) {
}
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] [PATCHv3] numatune: setting --mode does not work well
On 08/22/2014 04:05 PM, Erik Skultety wrote: When trying to set numatune mode directly using virsh numatune command, correct error is raised, however numatune structure was not deallocated, thus resulting in creating an empty numatune element in the guest XML, if none was present before. Running the same command aftewards results in a successful change with broken XML structure. Patch fixes the deallocation problem as well as checking for invalid attribute combination VIR_DOMAIN_NUMATUNE_PLACEMENT_AUTO + a nonempty nodeset. Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1129998 --- src/conf/numatune_conf.c | 45 ++ ...-numad-auto-vcpu-static-numatune-no-nodeset.xml | 31 +++ tests/qemuxml2argvtest.c | 1 + 3 files changed, 62 insertions(+), 15 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numad-auto-vcpu-static-numatune-no-nodeset.xml ACK and pushed 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] [PATCHv2] qemu: min_guarantee: Parameter 'min_guarantee' not supported
On 08/13/2014 10:07 AM, Erik Skultety wrote: The 'min_guarantee' is used by VMware ESX and OpenVZ drivers, with qemu however, libvirt should report error when starting a domain, because this element is not used. Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1122455 --- docs/formatdomain.html.in | 3 ++- src/qemu/qemu_process.c | 7 +++ 2 files changed, 9 insertions(+), 1 deletion(-) ACK and pushed Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Add support for boot menu timeout
This is basically just for qemu's '-boot splash-time' parameter. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1021703 Martin Kletzander (3): docs, conf: add support for bootmenu timeout qemu: add capability probing for splash-timeout qemu: add support for splash-timeout docs/formatdomain.html.in | 7 +- docs/schemas/domaincommon.rng | 5 src/conf/domain_conf.c | 21 ++-- src/conf/domain_conf.h | 2 ++ src/qemu/qemu_capabilities.c | 5 src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 15 +++ tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 + tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 + tests/qemuhelptest.c | 12 ++--- ...qemuxml2argv-boot-menu-disable-with-timeout.xml | 29 ++ ...2argv-boot-menu-enable-with-timeout-invalid.xml | 29 ++ ...qemuxml2argv-boot-menu-enable-with-timeout.args | 15 +++ .../qemuxml2argv-boot-menu-enable-with-timeout.xml | 29 ++ tests/qemuxml2argvtest.c | 5 ...muxml2xmlout-boot-menu-disable-with-timeout.xml | 29 ++ tests/qemuxml2xmltest.c| 2 ++ 18 files changed, 202 insertions(+), 7 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-disable-with-timeout.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout-invalid.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-boot-menu-disable-with-timeout.xml -- 2.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: add capability probing for splash-timeout
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_capabilities.c | 5 +
src/qemu/qemu_capabilities.h | 1 +
tests/qemucapabilitiesdata/caps_1.5.3-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.0-1.caps | 1 +
tests/qemucapabilitiesdata/caps_1.6.50-1.caps | 1 +
tests/qemuhelptest.c | 12
6 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index b758b5a..410086b 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -266,6 +266,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
memory-backend-file,
usb-audio,
rtc-reset-reinjection,
+
+ splash-timeout, /* 175 */
);
@@ -1132,6 +1134,8 @@ virQEMUCapsComputeCmdFlags(const char *help,
virQEMUCapsSet(qemuCaps, QEMU_CAPS_BOOT_MENU);
if (strstr(help, ,reboot-timeout=rb_time))
virQEMUCapsSet(qemuCaps, QEMU_CAPS_REBOOT_TIMEOUT);
+if (strstr(help, ,splash-time=sp_time))
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT);
if ((fsdev = strstr(help, -fsdev))) {
virQEMUCapsSet(qemuCaps, QEMU_CAPS_FSDEV);
if (strstr(fsdev, readonly))
@@ -2431,6 +2435,7 @@ static struct virQEMUCapsCommandLineProps
virQEMUCapsCommandLine[] = {
{ realtime, mlock, QEMU_CAPS_MLOCK },
{ boot-opts, strict, QEMU_CAPS_BOOT_STRICT },
{ boot-opts, reboot-timeout, QEMU_CAPS_REBOOT_TIMEOUT },
+{ boot-opts, splash-time, QEMU_CAPS_SPLASH_TIMEOUT },
{ spice, disable-agent-file-xfer, QEMU_CAPS_SPICE_FILE_XFER_DISABLE },
{ msg, timestamp, QEMU_CAPS_MSG_TIMESTAMP },
{ numa, NULL, QEMU_CAPS_NUMA },
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index cbd3646..48a4eaa 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -214,6 +214,7 @@ typedef enum {
QEMU_CAPS_OBJECT_MEMORY_FILE = 172, /* -object memory-backend-file */
QEMU_CAPS_OBJECT_USB_AUDIO = 173, /* usb-audio device support */
QEMU_CAPS_RTC_RESET_REINJECTION = 174, /* rtc-reset-reinjection monitor
command */
+QEMU_CAPS_SPLASH_TIMEOUT = 175, /* -boot splash-time */
QEMU_CAPS_LAST, /* this must always be the last item */
} virQEMUCapsFlags;
diff --git a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
index 5fa30aa..c220b46 100644
--- a/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.5.3-1.caps
@@ -138,4 +138,5 @@
flag name='usb-kbd'/
flag name='host-pci-multidomain'/
flag name='usb-audio'/
+flag name='splash-timeout'/
/qemuCaps
diff --git a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
index f364bbf..e10f030 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.0-1.caps
@@ -145,4 +145,5 @@
flag name='host-pci-multidomain'/
flag name='msg-timestamp'/
flag name='usb-audio'/
+flag name='splash-timeout'/
/qemuCaps
diff --git a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
index 10ce1b5..44f7b0c 100644
--- a/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
+++ b/tests/qemucapabilitiesdata/caps_1.6.50-1.caps
@@ -144,4 +144,5 @@
flag name='msg-timestamp'/
flag name='numa'/
flag name='usb-audio'/
+flag name='splash-timeout'/
/qemuCaps
diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c
index 366e36d..975edf3 100644
--- a/tests/qemuhelptest.c
+++ b/tests/qemuhelptest.c
@@ -745,7 +745,8 @@ mymain(void)
QEMU_CAPS_DEVICE_SCSI_GENERIC,
QEMU_CAPS_DEVICE_SCSI_GENERIC_BOOTINDEX,
QEMU_CAPS_DEVICE_USB_KBD,
-QEMU_CAPS_DEVICE_USB_STORAGE);
+QEMU_CAPS_DEVICE_USB_STORAGE,
+QEMU_CAPS_SPLASH_TIMEOUT);
DO_TEST(qemu-1.1.0, 1001000, 0, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -843,7 +844,8 @@ mymain(void)
QEMU_CAPS_VNC_SHARE_POLICY,
QEMU_CAPS_DEVICE_USB_KBD,
QEMU_CAPS_DEVICE_USB_STORAGE,
-QEMU_CAPS_OBJECT_USB_AUDIO);
+QEMU_CAPS_OBJECT_USB_AUDIO,
+QEMU_CAPS_SPLASH_TIMEOUT);
DO_TEST(qemu-1.2.0, 1002000, 0, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -953,7 +955,8 @@ mymain(void)
QEMU_CAPS_DEVICE_USB_STORAGE,
QEMU_CAPS_DEVICE_USB_KBD,
QEMU_CAPS_USB_STORAGE_REMOVABLE,
-QEMU_CAPS_OBJECT_USB_AUDIO);
+QEMU_CAPS_OBJECT_USB_AUDIO,
+QEMU_CAPS_SPLASH_TIMEOUT);
DO_TEST(qemu-kvm-1.2.0, 1002000, 1, 0,
QEMU_CAPS_VNC_COLON,
QEMU_CAPS_NO_REBOOT,
@@ -1068,7 +1071,8 @@ mymain(void)
QEMU_CAPS_DEVICE_USB_STORAGE,
[libvirt] [PATCH 1/3] docs, conf: add support for bootmenu timeout
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
docs/formatdomain.html.in | 7 +-
docs/schemas/domaincommon.rng | 5
src/conf/domain_conf.c | 21 ++--
src/conf/domain_conf.h | 2 ++
...qemuxml2argv-boot-menu-disable-with-timeout.xml | 29 ++
...2argv-boot-menu-enable-with-timeout-invalid.xml | 29 ++
.../qemuxml2argv-boot-menu-enable-with-timeout.xml | 29 ++
tests/qemuxml2argvtest.c | 1 +
...muxml2xmlout-boot-menu-disable-with-timeout.xml | 29 ++
tests/qemuxml2xmltest.c| 2 ++
10 files changed, 151 insertions(+), 3 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-boot-menu-disable-with-timeout.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout-invalid.xml
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-boot-menu-disable-with-timeout.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index ed17389..dedc4d1 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -105,7 +105,7 @@
lt;loadergt;/usr/lib/xen/boot/hvmloaderlt;/loadergt;
lt;boot dev='hd'/gt;
lt;boot dev='cdrom'/gt;
-lt;bootmenu enable='yes'/gt;
+lt;bootmenu enable='yes' timeout='3000'/gt;
lt;smbios mode='sysinfo'/gt;
lt;bios useserial='yes' rebootTimeout='0'/gt;
lt;/osgt;
@@ -158,6 +158,11 @@
startup. The codeenable/code attribute can be either yes or no.
If not specified, the hypervisor default is used. span class=since
Since 0.8.3/span
+ Additional attribute codetimeout/code takes the number of
milliseconds
+ the boot menu should wait until it times out. Allowed values are numbers
+ in range [0, 65535] inclusive and it is valid if and only if the previous
+ codeenable/code is set to yes.
+ span class=since Since 1.2.8/span
/dd
dtcodesmbios/code/dt
ddHow to populate SMBIOS information visible in the guest.
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 033f2f6..9a89dd8 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -259,6 +259,11 @@
valueno/value
/choice
/attribute
+optional
+ attribute name=timeout
+ref name=unsignedShort/
+ /attribute
+/optional
/element
/optional
optional
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9557020..6e4d602 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11188,6 +11188,19 @@ virDomainDefParseBootXML(xmlXPathContextPtr ctxt,
VIR_FREE(tmp);
}
+tmp = virXPathString(string(./os/bootmenu[1]/@timeout), ctxt);
+if (tmp def-os.bootmenu == VIR_TRISTATE_BOOL_YES) {
+if (virStrToLong_uip(tmp, NULL, 0, def-os.bm_timeout) 0 ||
+def-os.bm_timeout 65535) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(invalid value for boot menu timeout,
+ must be in range [0,65535]));
+goto cleanup;
+}
+def-os.bm_timeout_set = true;
+}
+VIR_FREE(tmp);
+
tmp = virXPathString(string(./os/bios[1]/@useserial), ctxt);
if (tmp) {
if (STREQ(tmp, yes)) {
@@ -17960,9 +17973,13 @@ virDomainDefFormatInternal(virDomainDefPtr def,
virBufferAsprintf(buf, boot dev='%s'/\n, boottype);
}
-if (def-os.bootmenu)
-virBufferAsprintf(buf, bootmenu enable='%s'/\n,
+if (def-os.bootmenu) {
+virBufferAsprintf(buf, bootmenu enable='%s',
virTristateBoolTypeToString(def-os.bootmenu));
+if (def-os.bm_timeout_set)
+virBufferAsprintf(buf, timeout='%u', def-os.bm_timeout);
+virBufferAddLit(buf, /\n);
+}
if (def-os.bios.useserial || def-os.bios.rt_set) {
virBufferAddLit(buf, bios);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f2df4eb..36ccf10 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1630,6 +1630,8 @@ struct _virDomainOSDef {
size_t nBootDevs;
int bootDevs[VIR_DOMAIN_BOOT_LAST];
int bootmenu; /* enum virTristateBool */
+unsigned int bm_timeout;
+bool bm_timeout_set;
char *init;
char **initargv;
char *kernel;
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-disable-with-timeout.xml
b/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-disable-with-timeout.xml
new file mode 100644
index 000..0f9bd0a
--- /dev/null
+++
Re: [libvirt] [PATCH v2 1/3] virfile: Refactor safezero, introduce virFileFdPosixFallocate
On Fri, Aug 22, 2014 at 10:56:47AM -0400, John Ferlan wrote:
On 08/22/2014 10:46 AM, Daniel P. Berrange wrote:
On Mon, Aug 11, 2014 at 04:30:19PM -0400, John Ferlan wrote:
Currently the safezero() function uses build conditionals to choose either
the posix_fallocate() or mmap() with a fallback to safewrite() in order to
preallocate a file.
This patch will modify the logic in order to allow fallbacks in the
event that posix_fallocate() or the ftruncate()and mmap() doesn't work
properly. The fallback will be to use the slow safewrite of zero filled
buffers to the file.
Have you actually encountered failing of posix_fallocate() in the
real world ? It is supposed to automatically fallback to the
equivalent of writing zeros if the filesystem / kernel does not
support it, so we should not have todo runtime fallback ourselves.
The existance of fallback is the main distinction between the
posix_fallocate() and fallocate() system calls.
It wasn't so much as a failure as unexpected results - the key being
that the resulting created (or resized) file was not sized as expected.
For an NFS target the results are not what was expected. I've left some
history in the prior set of patches with the following probably having
the most details:
http://www.redhat.com/archives/libvir-list/2014-August/msg00367.html
So, IIUC, the bug happens when the rsize mount option to NFS is not 4k.
strace'ing libvirtd on an NFS volume in this case shows:
open(/var/lib/libvirt/images/lettuce/foo, O_RDWR|O_CREAT|O_EXCL, 0600) = 24
fstat(24, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
ftruncate(24, 1073741824) = 0
fallocate(24, 0, 0, 1073741824) = -1 EOPNOTSUPP (Operation not
supported)
fallocate(24, 0, 0, 1073741824) = -1 EOPNOTSUPP (Operation not
supported)
fstat(24, {st_mode=S_IFREG|0600, st_size=1073741824, ...}) = 0
fstatfs(24, {f_type=NFS_SUPER_MAGIC, f_bsize=1048576, f_blocks=118342,
f_bfree=71002, f_bavail=65632, f_files=7678560, f_ffree=5495931, f_fsid={0, 0},
f_namelen=255, f_frsize=1048576}) = 0
pread(24, \0, 1, 1048575) = 1
pwrite(24, \0, 1, 1048575)= 1
pread(24, \0, 1, 2097151) = 1
pwrite(24, \0, 1, 2097151)= 1
pread(24, \0, 1, 3145727) = 1
So we can see glibc here trying fallocate() and then falling back to
writing zeros. Since the volume does not come out at the right size
this seems to show a bug in glibc.
So I think we really ought to report that bug to glibc to be fixed
there rather than working around it in libvirt, as there are many
more applications besides libvirt that will be impacted by this
bug.
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 3/3] qemu: add support for splash-timeout
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1021703
Signed-off-by: Martin Kletzander mklet...@redhat.com
---
src/qemu/qemu_command.c | 15 +++
.../qemuxml2argv-boot-menu-enable-with-timeout.args | 15 +++
tests/qemuxml2argvtest.c | 4
3 files changed, 34 insertions(+)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout.args
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 0d7b12d..bb1c423 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7927,6 +7927,21 @@ qemuBuildCommandLine(virConnectPtr conn,
def-os.bios.rt_delay);
}
+if (def-os.bm_timeout_set) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_SPLASH_TIMEOUT)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(splash timeout is not supported
+ by this QEMU binary));
+virBufferFreeAndReset(boot_buf);
+goto error;
+}
+
+if (boot_nparams++)
+virBufferAddChar(boot_buf, ',');
+
+virBufferAsprintf(boot_buf, splash-time=%d, def-os.bm_timeout);
+}
+
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_BOOT_STRICT)) {
if (boot_nparams++)
virBufferAddChar(boot_buf, ',');
diff --git
a/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout.args
b/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout.args
new file mode 100644
index 000..6141259
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-boot-menu-enable-with-timeout.args
@@ -0,0 +1,15 @@
+LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test QEMU_AUDIO_DRV=none \
+/usr/bin/qemu \
+-S \
+-M pc \
+-m 214 \
+-smp 1 \
+-nographic \
+-nodefaults \
+-monitor unix:/tmp/test-monitor,server,nowait \
+-no-acpi \
+-boot order=d,menu=on,splash-time=3000 \
+-usb \
+-drive file=/dev/cdrom,if=none,media=cdrom,id=drive-ide0-1-0 \
+-device ide-drive,bus=ide.1,unit=0,drive=drive-ide0-1-0,id=ide0-1-0 \
+-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index cfeec4f..97ffb3c 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -607,6 +607,10 @@ mymain(void)
DO_TEST(boot-menu-enable,
QEMU_CAPS_BOOT_MENU, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE,
QEMU_CAPS_BOOTINDEX);
+DO_TEST(boot-menu-enable-with-timeout,
+QEMU_CAPS_BOOT_MENU, QEMU_CAPS_DEVICE,
+QEMU_CAPS_DRIVE, QEMU_CAPS_SPLASH_TIMEOUT);
+DO_TEST_FAILURE(boot-menu-enable-with-timeout, QEMU_CAPS_BOOT_MENU);
DO_TEST_PARSE_ERROR(boot-menu-enable-with-timeout-invalid, NONE);
DO_TEST(boot-menu-disable, QEMU_CAPS_BOOT_MENU);
DO_TEST(boot-menu-disable-drive,
--
2.0.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2 1/3] virfile: Refactor safezero, introduce virFileFdPosixFallocate
On Fri, Aug 22, 2014 at 04:15:14PM +0100, Daniel P. Berrange wrote:
On Fri, Aug 22, 2014 at 10:56:47AM -0400, John Ferlan wrote:
On 08/22/2014 10:46 AM, Daniel P. Berrange wrote:
On Mon, Aug 11, 2014 at 04:30:19PM -0400, John Ferlan wrote:
Currently the safezero() function uses build conditionals to choose
either
the posix_fallocate() or mmap() with a fallback to safewrite() in order
to
preallocate a file.
This patch will modify the logic in order to allow fallbacks in the
event that posix_fallocate() or the ftruncate()and mmap() doesn't work
properly. The fallback will be to use the slow safewrite of zero filled
buffers to the file.
Have you actually encountered failing of posix_fallocate() in the
real world ? It is supposed to automatically fallback to the
equivalent of writing zeros if the filesystem / kernel does not
support it, so we should not have todo runtime fallback ourselves.
The existance of fallback is the main distinction between the
posix_fallocate() and fallocate() system calls.
It wasn't so much as a failure as unexpected results - the key being
that the resulting created (or resized) file was not sized as expected.
For an NFS target the results are not what was expected. I've left some
history in the prior set of patches with the following probably having
the most details:
http://www.redhat.com/archives/libvir-list/2014-August/msg00367.html
So, IIUC, the bug happens when the rsize mount option to NFS is not 4k.
strace'ing libvirtd on an NFS volume in this case shows:
open(/var/lib/libvirt/images/lettuce/foo, O_RDWR|O_CREAT|O_EXCL, 0600) = 24
fstat(24, {st_mode=S_IFREG|0600, st_size=0, ...}) = 0
ftruncate(24, 1073741824) = 0
fallocate(24, 0, 0, 1073741824) = -1 EOPNOTSUPP (Operation not
supported)
fallocate(24, 0, 0, 1073741824) = -1 EOPNOTSUPP (Operation not
supported)
fstat(24, {st_mode=S_IFREG|0600, st_size=1073741824, ...}) = 0
fstatfs(24, {f_type=NFS_SUPER_MAGIC, f_bsize=1048576, f_blocks=118342,
f_bfree=71002, f_bavail=65632, f_files=7678560, f_ffree=5495931, f_fsid={0,
0}, f_namelen=255, f_frsize=1048576}) = 0
pread(24, \0, 1, 1048575) = 1
pwrite(24, \0, 1, 1048575)= 1
pread(24, \0, 1, 2097151) = 1
pwrite(24, \0, 1, 2097151)= 1
pread(24, \0, 1, 3145727) = 1
So we can see glibc here trying fallocate() and then falling back to
writing zeros. Since the volume does not come out at the right size
this seems to show a bug in glibc.
So I think we really ought to report that bug to glibc to be fixed
there rather than working around it in libvirt, as there are many
more applications besides libvirt that will be impacted by this
bug.
Opps, meant to include the stack trace to show where the pread/writes
are coming from:
(gdb) bt
#0 pread64 () at ../sysdeps/unix/syscall-template.S:81
#1 0x7f55a29f9c5e in internal_fallocate (fd=fd@entry=24, offset=1048575,
len=1072693248)
at ../sysdeps/posix/posix_fallocate.c:78
#2 0x7f55a29f9cc7 in posix_fallocate (fd=fd@entry=24, offset=optimized
out, len=optimized out)
at ../sysdeps/unix/sysv/linux/wordsize-64/posix_fallocate.c:62
#3 0x7f55a6071026 in safezero (fd=fd@entry=24, offset=optimized out,
len=optimized out) at util/virfile.c:1031
#4 0x7f55916258c2 in createRawFile (inputvol=0x0, vol=0x7f5570008280,
fd=24) at storage/storage_backend.c:389
#5 virStorageBackendCreateRaw (conn=optimized out, pool=optimized out,
vol=0x7f5570008280, inputvol=0x0,
flags=optimized out) at storage/storage_backend.c:450
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 0/3] Resolve some Coverity issues
Resolve some issues seen by my daily Coverity run. Interesting that they didn't show up on the internal Jenkins Coverity scanner, but did show up on my daily run. Perhaps because I have 7.0.3 and the Jenkins has 7.0.0... I also see 7.5.0 is available - I'll give that a go too... John Ferlan (3): commandtest: Resolve Coverity RESOURCE_LEAK virnetsocket: Resolve Coverity RESOURCE_LEAK xenconfig: Resolve Coverity RESOURCE_LEAK src/rpc/virnetsocket.c | 8 ++-- src/xenconfig/xen_common.c | 2 ++ tests/commandtest.c| 1 + 3 files changed, 9 insertions(+), 2 deletions(-) -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/3] commandtest: Resolve Coverity RESOURCE_LEAK
Since '62f263a73' - Coverity complains if the !pidfile path is taken, then newfd1 would be leaked. Signed-off-by: John Ferlan jfer...@redhat.com --- tests/commandtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/commandtest.c b/tests/commandtest.c index b3287fa..5f52a00 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1081,6 +1081,7 @@ static int test24(const void *unused ATTRIBUTE_UNUSED) unlink(pidfile); VIR_FREE(pidfile); virCommandFree(cmd); +VIR_FORCE_CLOSE(newfd1); /* coverity[double_close] */ VIR_FORCE_CLOSE(newfd2); VIR_FORCE_CLOSE(newfd3); -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] virnetsocket: Resolve Coverity RESOURCE_LEAK
Since '1b807f92d' - Coverity complains that in the error paths of
both virFork() and virProcessWait() that the 'passfd' will not be closed
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/rpc/virnetsocket.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index f913365..ce908fa 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -593,8 +593,10 @@ int virNetSocketNewConnectUNIX(const char *path,
* behaviour on sockets according to POSIX, so it doesn't
* work outside Linux.
*/
-if ((pid = virFork()) 0)
+if ((pid = virFork()) 0) {
+VIR_FORCE_CLOSE(passfd);
goto error;
+}
if (pid == 0) {
umask(0077);
@@ -604,8 +606,10 @@ int virNetSocketNewConnectUNIX(const char *path,
_exit(EXIT_SUCCESS);
}
-if (virProcessWait(pid, status, false) 0)
+if (virProcessWait(pid, status, false) 0) {
+VIR_FORCE_CLOSE(passfd);
goto error;
+}
if (status != EXIT_SUCCESS) {
/*
--
1.9.3
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] xenconfig: Resolve Coverity RESOURCE_LEAK
Since '337a13628' - Coverity complains that 'net' is VIR_ALLOC()'d, but on various 'cleanup' exit paths from the code there is no corresponding cleanup. Since 'net' is eventually appended onto a list of nets we don't want to delete the last one - be sure to set it to NULL, but still call the free function in cleanup Signed-off-by: John Ferlan jfer...@redhat.com --- src/xenconfig/xen_common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 398e9ec..c487feb 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -949,6 +949,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) if (VIR_APPEND_ELEMENT(def-nets, def-nnets, net) 0) goto cleanup; +net = NULL; skipnic: list = list-next; @@ -960,6 +961,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) return 0; cleanup: +virDomainNetDefFree(net); VIR_FREE(script); return -1; } -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] commandtest: Resolve Coverity RESOURCE_LEAK
On 22.08.2014 17:28, John Ferlan wrote: Since '62f263a73' - Coverity complains if the !pidfile path is taken, then newfd1 would be leaked. Signed-off-by: John Ferlan jfer...@redhat.com --- tests/commandtest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/commandtest.c b/tests/commandtest.c index b3287fa..5f52a00 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1081,6 +1081,7 @@ static int test24(const void *unused ATTRIBUTE_UNUSED) unlink(pidfile); VIR_FREE(pidfile); virCommandFree(cmd); +VIR_FORCE_CLOSE(newfd1); /* coverity[double_close] */ VIR_FORCE_CLOSE(newfd2); VIR_FORCE_CLOSE(newfd3); ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] virnetsocket: Resolve Coverity RESOURCE_LEAK
On 22.08.2014 17:28, John Ferlan wrote:
Since '1b807f92d' - Coverity complains that in the error paths of
both virFork() and virProcessWait() that the 'passfd' will not be closed
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/rpc/virnetsocket.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index f913365..ce908fa 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -593,8 +593,10 @@ int virNetSocketNewConnectUNIX(const char *path,
* behaviour on sockets according to POSIX, so it doesn't
* work outside Linux.
*/
-if ((pid = virFork()) 0)
+if ((pid = virFork()) 0) {
+VIR_FORCE_CLOSE(passfd);
goto error;
+}
if (pid == 0) {
umask(0077);
@@ -604,8 +606,10 @@ int virNetSocketNewConnectUNIX(const char *path,
_exit(EXIT_SUCCESS);
}
-if (virProcessWait(pid, status, false) 0)
+if (virProcessWait(pid, status, false) 0) {
+VIR_FORCE_CLOSE(passfd);
goto error;
+}
if (status != EXIT_SUCCESS) {
/*
Unfortunately not enough context is shown to express myself, so I'll paste
interesting knobs from the function:
if (listen(passfd, 0) 0) {
virReportSystemError(errno, %s,
_(Failed to listen on socket that's about
to be passed to the daemon));
goto error;
}
if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
virReportSystemError(errno, _(Failed to connect socket to '%s'),
path);
goto error;
}
if (virNetSocketForkDaemon(binary, passfd) 0)
goto error;
}
localAddr.len = sizeof(localAddr.data);
if (getsockname(fd, localAddr.data.sa, localAddr.len) 0) {
virReportSystemError(errno, %s, _(Unable to get local socket
name));
goto error;
}
if (!(*retsock = virNetSocketNew(localAddr, remoteAddr, true, fd, -1,
0)))
goto error;
return 0;
error:
VIR_FREE(buf);
VIR_FORCE_CLOSE(fd);
if (spawnDaemon)
unlink(path);
return -1;
}
Here, if any of listen() or connect() fail, the control continues on the error
label and the @passfd is leaked again.
virNetSocketForkDaemon() is different - it closes passfd on failure. So I
suggest moving VIR_FORCE_CLOSE() under the error label.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] xenconfig: Resolve Coverity RESOURCE_LEAK
On 22.08.2014 17:28, John Ferlan wrote: Since '337a13628' - Coverity complains that 'net' is VIR_ALLOC()'d, but on various 'cleanup' exit paths from the code there is no corresponding cleanup. Since 'net' is eventually appended onto a list of nets we don't want to delete the last one - be sure to set it to NULL, but still call the free function in cleanup Signed-off-by: John Ferlan jfer...@redhat.com --- src/xenconfig/xen_common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 398e9ec..c487feb 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -949,6 +949,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) if (VIR_APPEND_ELEMENT(def-nets, def-nnets, net) 0) goto cleanup; +net = NULL; This is not needed. VIR_APPEND_ELEMENT() should clear out the @net variable. Or does coverity fail to see it? skipnic: list = list-next; @@ -960,6 +961,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) return 0; cleanup: +virDomainNetDefFree(net); VIR_FREE(script); return -1; } In fact this chunk alone should be enough. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/2] parallels: build with parallels SDK
On Friday 22 August 2014 16:38:17 Michal Privoznik wrote: On 21.08.2014 22:36, Dmitry Guryanov wrote: Executing prlctl command is not an optimal way to interact with Parallels Cloud Server (PCS), it's better to use parallels SDK, which is a remote API to paralles dispatcher service. We prepared opensource version of this SDK and published it on github, it's distributed under LGPL license. Here is a git repo: https://github.com/Parallels/parallels-sdk. To build with parallels SDK user should get compiler and linker options from pkg-config 'parallels-sdk' file. So fix checks in configure script and build with parallels SDK, if that pkg-config file exists and add gcc options to makefile. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- configure.ac| 11 +-- src/Makefile.am | 4 +++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index f93c6c2..4318d24 100644 --- a/configure.ac +++ b/configure.ac @@ -1046,15 +1046,14 @@ dnl dnl Checks for the Parallels driver dnl +PKG_CHECK_MODULES([PARALLELS_SDK], [parallels-sdk], [PARALLELS_SDK_FOUND=yes], [PARALLELS_SDK_FOUND=no]) + if test $with_parallels = check; then -with_parallels=$with_linux -if test ! $host_cpu = 'x86_64'; then -with_parallels=no -fi +with_parallels=$PARALLELS_SDK_FOUND fi -if test $with_parallels = yes test $with_linux = no; then -AC_MSG_ERROR([The Parallels driver can be enabled on Linux only.]) +if test $with_parallels = yes test $PARALLELS_SDK_FOUND = no; then +AC_MSG_ERROR([Parallels Virtualization SDK is needed to build the Parallels driver.]) fi if test $with_parallels = yes; then diff --git a/src/Makefile.am b/src/Makefile.am index 538530e..dad7c7f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1352,7 +1352,9 @@ if WITH_PARALLELS noinst_LTLIBRARIES += libvirt_driver_parallels.la libvirt_la_BUILT_LIBADD += libvirt_driver_parallels.la libvirt_driver_parallels_la_CFLAGS = \ - -I$(top_srcdir)/src/conf $(AM_CFLAGS) + -I$(top_srcdir)/src/conf $(AM_CFLAGS) \ + $(PARALLELS_SDK_CFLAGS) +libvirt_driver_parallels_la_LIBADD = $(PARALLELS_SDK_LIBS) libvirt_driver_parallels_la_SOURCES = $(PARALLELS_DRIVER_SOURCES) endif WITH_PARALLELS So previously, this was enabled for x86_64 linux hosts, now you need to have SDK. I understand why you are doing that but perhaps do we want to enable backward compatibility and let users use prlctl command? Actually backward compatibility is not needed, because there was only several users inside company. Also PCS is a full distribution to be installed on baremetal hosts, so starting with the next update we will provide this new version of SDK and libvirt package, built with parallels support. Users, which connect to PCS from their desktops will use parallels+ssh scheme and still will be able to use this driver. Michal -- Dmitry Guryanov -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] virnetsocket: Resolve Coverity RESOURCE_LEAK
On 08/22/2014 05:28 PM, John Ferlan wrote:
Since '1b807f92d' - Coverity complains that in the error paths of
both virFork() and virProcessWait() that the 'passfd' will not be closed
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/rpc/virnetsocket.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index f913365..ce908fa 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -593,8 +593,10 @@ int virNetSocketNewConnectUNIX(const char *path,
* behaviour on sockets according to POSIX, so it doesn't
* work outside Linux.
*/
-if ((pid = virFork()) 0)
+if ((pid = virFork()) 0) {
+VIR_FORCE_CLOSE(passfd);
goto error;
+}
if (pid == 0) {
umask(0077);
@@ -604,8 +606,10 @@ int virNetSocketNewConnectUNIX(const char *path,
_exit(EXIT_SUCCESS);
}
-if (virProcessWait(pid, status, false) 0)
+if (virProcessWait(pid, status, false) 0) {
+VIR_FORCE_CLOSE(passfd);
goto error;
+}
if (status != EXIT_SUCCESS) {
/*
Unless I'm missing something, passfd will be leaked on all error paths unless
virNetSocketForkDaemon succeeds.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] commandtest: Resolve Coverity RESOURCE_LEAK
On 08/22/2014 05:28 PM, John Ferlan wrote: Since '62f263a73' - Coverity complains if the !pidfile path is taken, then newfd1 would be leaked. Signed-off-by: John Ferlan jfer...@redhat.com --- tests/commandtest.c | 1 + 1 file changed, 1 insertion(+) ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] xenconfig: Resolve Coverity RESOURCE_LEAK
On 08/22/2014 05:28 PM, John Ferlan wrote: Since '337a13628' - Coverity complains that 'net' is VIR_ALLOC()'d, but on various 'cleanup' exit paths from the code there is no corresponding cleanup. Since 'net' is eventually appended onto a list of nets we don't want to delete the last one - be sure to set it to NULL, but still call the free function in cleanup Signed-off-by: John Ferlan jfer...@redhat.com --- src/xenconfig/xen_common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 398e9ec..c487feb 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -949,6 +949,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) if (VIR_APPEND_ELEMENT(def-nets, def-nnets, net) 0) goto cleanup; +net = NULL; This bit should not be necessary. VIR_APPEND_ELEMENT expands to virInsertElementsN with clearOriginal=true skipnic: list = list-next; @@ -960,6 +961,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) return 0; cleanup: +virDomainNetDefFree(net); VIR_FREE(script); return -1; } ACK Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] parallels: login to parallels SDK
On Friday 22 August 2014 16:38:11 Michal Privoznik wrote:
On 21.08.2014 22:36, Dmitry Guryanov wrote:
Add files parallels_sdk.c and parallels_sdk.h for code
which works with SDK, so libvirt's code will not mix with
dealing with parallels SDK.
To use Parallels SDK you must first call PrlApi_InitEx function,
and then you will be able to connect to a server with
PrlSrv_LoginLocalEx function. When you've done you must call
PrlApi_Deinit. So let's call PrlApi_InitEx on first .connectOpen,
count number of connections and deinitialize, when this counter
becomes zero.
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
po/POTFILES.in | 1 +
src/Makefile.am | 4 +++-
src/parallels/parallels_driver.c | 46
+++-
src/parallels/parallels_utils.h | 3 +++
4 files changed, 52 insertions(+), 2 deletions(-)
This fails 'make syntax-check' for me.
diff --git a/po/POTFILES.in b/po/POTFILES.in
index f17b35f..4c1302d 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -96,6 +96,7 @@ src/openvz/openvz_driver.c
src/openvz/openvz_util.c
src/parallels/parallels_driver.c
src/parallels/parallels_network.c
+src/parallels/parallels_sdk.c
src/parallels/parallels_utils.c
src/parallels/parallels_utils.h
src/parallels/parallels_storage.c
diff --git a/src/Makefile.am b/src/Makefile.am
index dad7c7f..d4c6465 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -787,7 +787,9 @@ PARALLELS_DRIVER_SOURCES =
\
parallels/parallels_utils.c \
parallels/parallels_utils.h \
parallels/parallels_storage.c \
- parallels/parallels_network.c
+ parallels/parallels_network.c \
+ parallels/parallels_sdk.h \
+ parallels/parallels_sdk.c
BHYVE_DRIVER_SOURCES =
\
bhyve/bhyve_capabilities.c
\
diff --git a/src/parallels/parallels_driver.c
b/src/parallels/parallels_driver.c index bb9538f..7dc9963 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -55,6 +55,7 @@
#include parallels_driver.h
#include parallels_utils.h
+#include parallels_sdk.h
#define VIR_FROM_THIS VIR_FROM_PARALLELS
@@ -73,6 +74,9 @@ VIR_LOG_INIT(parallels.parallels_driver);
#define IS_CT(def) (STREQ_NULLABLE(def-os.type, exe))
+unsigned int numConns = 0;
+virMutex numConnsLock;
1: ^^^
+
static int parallelsConnectClose(virConnectPtr conn);
static const char * parallelsGetDiskBusName(int bus) {
@@ -929,9 +933,25 @@ parallelsOpenDefault(virConnectPtr conn)
if (virMutexInit(privconn-lock) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(cannot initialize mutex));
-goto error;
+goto err_free;
}
+virMutexLock(numConnsLock);
+numConns++;
+
+if (numConns == 1) {
+if (prlsdkInit()) {
+VIR_DEBUG(%s, _(Can't initialize Parallels SDK));
+virMutexUnlock(numConnsLock);
+goto err_free;
+}
+}
+
+virMutexUnlock(numConnsLock);
+
+if (prlsdkConnect(privconn) 0)
+goto err_free;
+
if (!(privconn-caps = parallelsBuildCapabilities()))
goto error;
@@ -953,6 +973,9 @@ parallelsOpenDefault(virConnectPtr conn)
virObjectUnref(privconn-domains);
virObjectUnref(privconn-caps);
virStoragePoolObjListFree(privconn-pools);
+prlsdkDisconnect(privconn);
+prlsdkDeinit();
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/3] conf: Extend loader/ and introduce nvram/
On 08/21/2014 02:50 AM, Michal Privoznik wrote: Up to now, users can configure BIOS via the loader/ element. With the upcoming implementation of UEFI this is not enough as BIOS and UEFI are conceptually different. For instance, while BIOS is ROM, UEFI is programmable flash (although all writes to code section are denied). Therefore we need new attribute @type which will differentiate the two. Then, new attribute @readonly is introduced to reflect the fact that some images are RO. Moreover, the OVMF (which is going to be used mostly), works in two modes: 1) Code and UEFI variable store is mixed in one file. 2) Code and UEFI variable store is separated in two files The latter has advantage of updating the UEFI code without losing the configuration. However, in order to represent the latter case we need yet another XML element: nvram/. Currently, it has no additional attributes, it's just a bare element containing path to the variable store file. +++ b/docs/formatdomain.html.in @@ -102,7 +102,8 @@ ... lt;osgt; lt;typegt;hvmlt;/typegt; -lt;loadergt;/usr/lib/xen/boot/hvmloaderlt;/loadergt; +lt;loader readonly='on' type='rom'gt;/usr/lib/xen/boot/hvmloaderlt;/loadergt; readonly='yes' is a bit more typical of other XML constructs. +lt;nvramgt;/var/lib/libvirt/nvram/guest_VARS.fdlt;/nvramgt; You chose nvram to be a sibling, rather than a child, of loader. Is it legal to have nvram in isolation, or can it only appear when loader is present? If the former, then you are okay; if the latter, then I'd rather see it as a child than a sibling. lt;boot dev='hd'/gt; lt;boot dev='cdrom'/gt; lt;bootmenu enable='yes'/gt; @@ -129,7 +130,21 @@ used to assist the domain creation process. It is used by Xen fully virtualized domains as well as setting the QEMU BIOS file path for QEMU/KVM domains. span class=sinceXen since 0.1.0, -QEMU/KVM since 0.9.12/span/dd +QEMU/KVM since 0.9.12/span Then, span class=sinceSince s/Since/since/, because you are using it in the middle of a sentence +1.2.8/span it's possible for the element to have two +optional attributes: codereadonly/code (accepted values are +codeon/code and codeoff/code) to reflect the fact that the +image should be writable or read-only. Again, yes/no might be more consistent than on/off The second attribute +codetype/code accepts values coderom/code and +codepflash/code. It tells the hypervisor where in the guest +memory the file should be mapped. For instance, if the loader +path points to an UEFI image, codetype/code should be +codepflash/code./dd + dtcodenvram/code/dt + ddSome UEFI firmwares may want to use a non-volatile memory to store +some variables. In the host, this is represented as a file and the +path to the file is stored in this element. span class=sinceSince +1.2.8/span/dd Is this going to bite us in the future? What if we want to store the file on a networked device, such as via gluster or nbd? I'm wondering if: nvram type='file' source file='/path/to/storage'/ /nvram is a better representation, because that way, we could also do: nvram type='network' source protocol='gluster' ... !-- all the stuff we do for disk type='network' -- /source /nvram Also, by reusing a virStorageSourcePtr to store the nvram location, rather than limiting to just a file name, we can ensure we do proper SELinux labeling of the file, and allow the user the ability to overwrite what label we would otherwise generate. +++ b/docs/schemas/domaincommon.rng @@ -242,6 +242,27 @@ interleave optional element name=loader +optional + attribute name=readonly +choice + valueon/value + valueoff/value +/choice + /attribute +/optional +optional + attribute name=type +choice + valuerom/value + valuepflash/value +/choice + /attribute +/optional +ref name=absFilePath/ + /element +/optional +optional + element name=nvram ref name=absFilePath/ This matches your docs, whether or not we decide to change the design. And at any rate, the existing loader element is just as hard-coded to a local filename as your new nvram, so if we add type='file' support, we'd want it on both places at once, so probably fine to defer that to a later day if someone actually needs it. So for now, I'm okay living with the design of just a string (but would still like to see the on/off changed to yes/no), since it's better to get OVMF usage enabled than to wait for a more complicated virStorageSource solution.
Re: [libvirt] [PATCH 2/3] virnetsocket: Resolve Coverity RESOURCE_LEAK
On 08/22/2014 11:49 AM, Michal Privoznik wrote:
On 22.08.2014 17:28, John Ferlan wrote:
Since '1b807f92d' - Coverity complains that in the error paths of
both virFork() and virProcessWait() that the 'passfd' will not be closed
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/rpc/virnetsocket.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index f913365..ce908fa 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -593,8 +593,10 @@ int virNetSocketNewConnectUNIX(const char *path,
* behaviour on sockets according to POSIX, so it doesn't
* work outside Linux.
*/
-if ((pid = virFork()) 0)
+if ((pid = virFork()) 0) {
+VIR_FORCE_CLOSE(passfd);
goto error;
+}
if (pid == 0) {
umask(0077);
@@ -604,8 +606,10 @@ int virNetSocketNewConnectUNIX(const char *path,
_exit(EXIT_SUCCESS);
}
-if (virProcessWait(pid, status, false) 0)
+if (virProcessWait(pid, status, false) 0) {
+VIR_FORCE_CLOSE(passfd);
goto error;
+}
if (status != EXIT_SUCCESS) {
/*
Unfortunately not enough context is shown to express myself, so I'll paste
interesting knobs from the function:
if (listen(passfd, 0) 0) {
virReportSystemError(errno, %s,
_(Failed to listen on socket that's about
to be passed to the daemon));
goto error;
}
if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
virReportSystemError(errno, _(Failed to connect socket to
'%s'),
path);
goto error;
}
if (virNetSocketForkDaemon(binary, passfd) 0)
goto error;
}
localAddr.len = sizeof(localAddr.data);
if (getsockname(fd, localAddr.data.sa, localAddr.len) 0) {
virReportSystemError(errno, %s, _(Unable to get local socket
name));
goto error;
}
if (!(*retsock = virNetSocketNew(localAddr, remoteAddr, true, fd, -1,
0)))
goto error;
return 0;
error:
VIR_FREE(buf);
VIR_FORCE_CLOSE(fd);
if (spawnDaemon)
unlink(path);
return -1;
}
Here, if any of listen() or connect() fail, the control continues on the
error label and the @passfd is leaked again.
virNetSocketForkDaemon() is different - it closes passfd on failure. So I
suggest moving VIR_FORCE_CLOSE() under the error label.
Yeah - Coverity only complained about the two paths - so I was hesitant
to put the VIR_FORCE_CLOSE(passfd) in the error: path...
In any case, putting it the error: path does resolve the issue as well
as making sure to initialize passfd = -1 (initializing fd = -1 wasn't
necessary since there's currently no way to error unless it's set)
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] xenconfig: Resolve Coverity RESOURCE_LEAK
On 08/22/2014 11:28 AM, John Ferlan wrote: Since '337a13628' - Coverity complains that 'net' is VIR_ALLOC()'d, but on various 'cleanup' exit paths from the code there is no corresponding cleanup. Since 'net' is eventually appended onto a list of nets we don't want to delete the last one - be sure to set it to NULL, but still call the free function in cleanup Signed-off-by: John Ferlan jfer...@redhat.com --- src/xenconfig/xen_common.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/xenconfig/xen_common.c b/src/xenconfig/xen_common.c index 398e9ec..c487feb 100644 --- a/src/xenconfig/xen_common.c +++ b/src/xenconfig/xen_common.c @@ -949,6 +949,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) if (VIR_APPEND_ELEMENT(def-nets, def-nnets, net) 0) goto cleanup; +net = NULL; OK - so removing this worked fine - my brain certainly thought it was necessary based on previous times I've made coverity changes... John skipnic: list = list-next; @@ -960,6 +961,7 @@ xenParseVif(virConfPtr conf, virDomainDefPtr def) return 0; cleanup: +virDomainNetDefFree(net); VIR_FREE(script); return -1; } -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/3] conf: Extend loader/ and introduce nvram/
On Fri, Aug 22, 2014 at 10:41:13AM -0600, Eric Blake wrote: On 08/21/2014 02:50 AM, Michal Privoznik wrote: Up to now, users can configure BIOS via the loader/ element. With the upcoming implementation of UEFI this is not enough as BIOS and UEFI are conceptually different. For instance, while BIOS is ROM, UEFI is programmable flash (although all writes to code section are denied). Therefore we need new attribute @type which will differentiate the two. Then, new attribute @readonly is introduced to reflect the fact that some images are RO. Moreover, the OVMF (which is going to be used mostly), works in two modes: 1) Code and UEFI variable store is mixed in one file. 2) Code and UEFI variable store is separated in two files The latter has advantage of updating the UEFI code without losing the configuration. However, in order to represent the latter case we need yet another XML element: nvram/. Currently, it has no additional attributes, it's just a bare element containing path to the variable store file. +++ b/docs/formatdomain.html.in @@ -102,7 +102,8 @@ ... lt;osgt; lt;typegt;hvmlt;/typegt; -lt;loadergt;/usr/lib/xen/boot/hvmloaderlt;/loadergt; +lt;loader readonly='on' type='rom'gt;/usr/lib/xen/boot/hvmloaderlt;/loadergt; readonly='yes' is a bit more typical of other XML constructs. +lt;nvramgt;/var/lib/libvirt/nvram/guest_VARS.fdlt;/nvramgt; You chose nvram to be a sibling, rather than a child, of loader. Is it legal to have nvram in isolation, or can it only appear when loader is present? If the former, then you are okay; if the latter, then I'd rather see it as a child than a sibling. loader is a long standing element whose contents is a string path. So from a back compatibility POV we can't make nvram be a child of that, even though it would make sense. 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 2/2] parallels: login to parallels SDK
On Friday 22 August 2014 16:38:11 Michal Privoznik wrote:
On 21.08.2014 22:36, Dmitry Guryanov wrote:
Add files parallels_sdk.c and parallels_sdk.h for code
which works with SDK, so libvirt's code will not mix with
dealing with parallels SDK.
To use Parallels SDK you must first call PrlApi_InitEx function,
and then you will be able to connect to a server with
PrlSrv_LoginLocalEx function. When you've done you must call
PrlApi_Deinit. So let's call PrlApi_InitEx on first .connectOpen,
count number of connections and deinitialize, when this counter
becomes zero.
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
po/POTFILES.in | 1 +
src/Makefile.am | 4 +++-
src/parallels/parallels_driver.c | 46
+++-
src/parallels/parallels_utils.h | 3 +++
4 files changed, 52 insertions(+), 2 deletions(-)
This fails 'make syntax-check' for me.
It seems I've sent completely wrong patch :( I should add 2 more files in this
patch.
diff --git a/po/POTFILES.in b/po/POTFILES.in
index f17b35f..4c1302d 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -96,6 +96,7 @@ src/openvz/openvz_driver.c
src/openvz/openvz_util.c
src/parallels/parallels_driver.c
src/parallels/parallels_network.c
+src/parallels/parallels_sdk.c
src/parallels/parallels_utils.c
src/parallels/parallels_utils.h
src/parallels/parallels_storage.c
diff --git a/src/Makefile.am b/src/Makefile.am
index dad7c7f..d4c6465 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -787,7 +787,9 @@ PARALLELS_DRIVER_SOURCES =
\
parallels/parallels_utils.c \
parallels/parallels_utils.h \
parallels/parallels_storage.c \
- parallels/parallels_network.c
+ parallels/parallels_network.c \
+ parallels/parallels_sdk.h \
+ parallels/parallels_sdk.c
BHYVE_DRIVER_SOURCES =
\
bhyve/bhyve_capabilities.c
\
diff --git a/src/parallels/parallels_driver.c
b/src/parallels/parallels_driver.c index bb9538f..7dc9963 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -55,6 +55,7 @@
#include parallels_driver.h
#include parallels_utils.h
+#include parallels_sdk.h
#define VIR_FROM_THIS VIR_FROM_PARALLELS
@@ -73,6 +74,9 @@ VIR_LOG_INIT(parallels.parallels_driver);
#define IS_CT(def) (STREQ_NULLABLE(def-os.type, exe))
+unsigned int numConns = 0;
+virMutex numConnsLock;
1: ^^^
+
static int parallelsConnectClose(virConnectPtr conn);
static const char * parallelsGetDiskBusName(int bus) {
@@ -929,9 +933,25 @@ parallelsOpenDefault(virConnectPtr conn)
if (virMutexInit(privconn-lock) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(cannot initialize mutex));
-goto error;
+goto err_free;
}
+virMutexLock(numConnsLock);
+numConns++;
+
+if (numConns == 1) {
+if (prlsdkInit()) {
+VIR_DEBUG(%s, _(Can't initialize Parallels SDK));
+virMutexUnlock(numConnsLock);
+goto err_free;
+}
+}
+
+virMutexUnlock(numConnsLock);
+
+if (prlsdkConnect(privconn) 0)
+goto err_free;
+
if (!(privconn-caps = parallelsBuildCapabilities()))
goto error;
@@ -953,6 +973,9 @@ parallelsOpenDefault(virConnectPtr conn)
virObjectUnref(privconn-domains);
virObjectUnref(privconn-caps);
virStoragePoolObjListFree(privconn-pools);
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 2/3] qemu: Implement extended loader and nvram
On 08/21/2014 02:50 AM, Michal Privoznik wrote:
QEMU now supports UEFI with the following command line:
-drive
file=/usr/share/OVMF/OVMF_CODE.fd,if=pflash,format=raw,unit=0,readonly=on \
-drive file=/usr/share/OVMF/OVMF_VARS.fd,if=pflash,format=raw,unit=1 \
where the first line reflects loader and the second one nvram.
Moreover, these two lines obsoletes the -bios argument.
s/obsoletes/obsolete/
Note that UEFI is unusable without ACPI. This is handled properly now.
Among with this extension, the variable file is expected to be
writable and hence we need security drivers to label it.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
+case VIR_DOMAIN_LOADER_TYPE_PFLASH:
+/* UEFI is supported only for x86_64 currently */
+if (def-os.arch != VIR_ARCH_X86_64) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _(pflash is not supported for %s guest
achitecture),
s/achitecture/architecture/
+
+if (loader-readonly) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DRIVE_READONLY)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
+ _(this qemu doesn't support passing
+ readonly attribute));
+goto cleanup;
+}
+
+virBufferAsprintf(buf, ,readonly=%s,
+
virTristateSwitchTypeToString(loader-readonly));
+}
+
+virCommandAddArg(cmd, -drive);
+virCommandAddArgBuffer(cmd, buf);
+
+if (loader-nvram) {
+virBufferFreeAndReset(buf);
+virBufferAsprintf(buf,
+ file=%s,if=pflash,format=raw,unit=%d,
+ loader-nvram, unit);
Hmm. Here, it looks like readonly='on' is supported ONLY for
type='pflash', and not for type='rom'. In that case, I'd write the .rng
of patch 1 as (rough draft):
element name='loader'
choice
group !-- bios, default loader type --
optional
attribute name='type'
valuerom/value
/attribute
/optional
/group
group !-- pflash, for OVMF use --
attribute name='type'
valuepflash/value
/attribute
optional
attribute name='readonly'...
/optional
optional
nvram stuff...
/optional
/group
/choice
ref name='absFileName'
/element
--
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] virnetsocket: Resolve Coverity RESOURCE_LEAK
On 08/22/2014 12:42 PM, John Ferlan wrote:
On 08/22/2014 11:49 AM, Michal Privoznik wrote:
On 22.08.2014 17:28, John Ferlan wrote:
Since '1b807f92d' - Coverity complains that in the error paths of
both virFork() and virProcessWait() that the 'passfd' will not be closed
Signed-off-by: John Ferlan jfer...@redhat.com
---
src/rpc/virnetsocket.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index f913365..ce908fa 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -593,8 +593,10 @@ int virNetSocketNewConnectUNIX(const char *path,
* behaviour on sockets according to POSIX, so it doesn't
* work outside Linux.
*/
-if ((pid = virFork()) 0)
+if ((pid = virFork()) 0) {
+VIR_FORCE_CLOSE(passfd);
goto error;
+}
if (pid == 0) {
umask(0077);
@@ -604,8 +606,10 @@ int virNetSocketNewConnectUNIX(const char *path,
_exit(EXIT_SUCCESS);
}
-if (virProcessWait(pid, status, false) 0)
+if (virProcessWait(pid, status, false) 0) {
+VIR_FORCE_CLOSE(passfd);
goto error;
+}
if (status != EXIT_SUCCESS) {
/*
Unfortunately not enough context is shown to express myself, so I'll paste
interesting knobs from the function:
if (listen(passfd, 0) 0) {
virReportSystemError(errno, %s,
_(Failed to listen on socket that's about
to be passed to the daemon));
goto error;
}
if (connect(fd, remoteAddr.data.sa, remoteAddr.len) 0) {
virReportSystemError(errno, _(Failed to connect socket to
'%s'),
path);
goto error;
}
if (virNetSocketForkDaemon(binary, passfd) 0)
goto error;
}
localAddr.len = sizeof(localAddr.data);
if (getsockname(fd, localAddr.data.sa, localAddr.len) 0) {
virReportSystemError(errno, %s, _(Unable to get local socket
name));
goto error;
}
if (!(*retsock = virNetSocketNew(localAddr, remoteAddr, true, fd, -1,
0)))
goto error;
return 0;
error:
VIR_FREE(buf);
While implementing the change to add passfd here - I noticed that buf is
never used in this context? Strange. There's an initialization to NULL
and a VIR_FREE(buf);, but nothing else uses it, so I removed it.
John
VIR_FORCE_CLOSE(fd);
if (spawnDaemon)
unlink(path);
return -1;
}
Here, if any of listen() or connect() fail, the control continues on the
error label and the @passfd is leaked again.
virNetSocketForkDaemon() is different - it closes passfd on failure. So I
suggest moving VIR_FORCE_CLOSE() under the error label.
Yeah - Coverity only complained about the two paths - so I was hesitant
to put the VIR_FORCE_CLOSE(passfd) in the error: path...
In any case, putting it the error: path does resolve the issue as well
as making sure to initialize passfd = -1 (initializing fd = -1 wasn't
necessary since there's currently no way to error unless it's set)
John
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v4 1/3] conf: Extend loader/ and introduce nvram/
On 08/22/2014 10:43 AM, Daniel P. Berrange wrote: lt;osgt; lt;typegt;hvmlt;/typegt; -lt;loadergt;/usr/lib/xen/boot/hvmloaderlt;/loadergt; +lt;loader readonly='on' type='rom'gt;/usr/lib/xen/boot/hvmloaderlt;/loadergt; readonly='yes' is a bit more typical of other XML constructs. +lt;nvramgt;/var/lib/libvirt/nvram/guest_VARS.fdlt;/nvramgt; You chose nvram to be a sibling, rather than a child, of loader. Is it legal to have nvram in isolation, or can it only appear when loader is present? If the former, then you are okay; if the latter, then I'd rather see it as a child than a sibling. loader is a long standing element whose contents is a string path. So from a back compatibility POV we can't make nvram be a child of that, even though it would make sense. Hmm. But what if we allow a choice between: loader type='rom'/path/to/rom/loader and loader type='pflash' config/path/to/config/config nvram/path/to/nvram/nvram /loader that is, use the (optional) type='rom|pflash' for gating whether the rest of the loader element is a single name (old style) or structured layout (new style)? -- 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 v4 1/3] conf: Extend loader/ and introduce nvram/
On Fri, Aug 22, 2014 at 10:51:18AM -0600, Eric Blake wrote: On 08/22/2014 10:43 AM, Daniel P. Berrange wrote: lt;osgt; lt;typegt;hvmlt;/typegt; -lt;loadergt;/usr/lib/xen/boot/hvmloaderlt;/loadergt; +lt;loader readonly='on' type='rom'gt;/usr/lib/xen/boot/hvmloaderlt;/loadergt; readonly='yes' is a bit more typical of other XML constructs. +lt;nvramgt;/var/lib/libvirt/nvram/guest_VARS.fdlt;/nvramgt; You chose nvram to be a sibling, rather than a child, of loader. Is it legal to have nvram in isolation, or can it only appear when loader is present? If the former, then you are okay; if the latter, then I'd rather see it as a child than a sibling. loader is a long standing element whose contents is a string path. So from a back compatibility POV we can't make nvram be a child of that, even though it would make sense. Hmm. But what if we allow a choice between: loader type='rom'/path/to/rom/loader and loader type='pflash' config/path/to/config/config nvram/path/to/nvram/nvram /loader that is, use the (optional) type='rom|pflash' for gating whether the rest of the loader element is a single name (old style) or structured layout (new style)? That is still going to cause existing apps which are parsing the XML to fail due to the change in child content. Changing content from a text node to elements is something I don't think we can do with the XML schemas. 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 0/3] Resolve some Coverity issues
On 08/22/2014 11:28 AM, John Ferlan wrote: Resolve some issues seen by my daily Coverity run. Interesting that they didn't show up on the internal Jenkins Coverity scanner, but did show up on my daily run. Perhaps because I have 7.0.3 and the Jenkins has 7.0.0... I also see 7.5.0 is available - I'll give that a go too... John Ferlan (3): commandtest: Resolve Coverity RESOURCE_LEAK virnetsocket: Resolve Coverity RESOURCE_LEAK xenconfig: Resolve Coverity RESOURCE_LEAK src/rpc/virnetsocket.c | 8 ++-- src/xenconfig/xen_common.c | 2 ++ tests/commandtest.c| 1 + 3 files changed, 9 insertions(+), 2 deletions(-) I've resolved the points from review and pushed. Thanks for the quick reviews! John FWIW: I should never have upgraded my coverity to 7.5.0 - it's *way too* chatty! sigh Although it seems to have found some more interesting latent things, but also it seems a new set of false positives. double sigh... Only 164 things to look at! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix spacing around commas
On 08/22/2014 07:14 AM, Michal Privoznik wrote: On some places in the libvirt code we have: f(a,z) instead of f(a, z) This trivial patch fixes couple of such occurrences. Signed-off-by: Michal Privoznik mpriv...@redhat.com --- Pushed under trivial rule. Do we have a syntax-check to guarantee we don't regress? -- 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] [PATCH v2 0/2] parallels: use parallels SDK instead of prlctl tool
This patchset begins reworking of parallels driver. We have published Opensource version of parallels SDK (under LGPL license), so libvirt can link with it. Dmitry Guryanov (2): parallels: build with parallels SDK parallels: login to parallels SDK configure.ac | 11 +- po/POTFILES.in | 1 + src/Makefile.am | 8 +- src/parallels/parallels_driver.c | 40 ++- src/parallels/parallels_sdk.c| 234 +++ src/parallels/parallels_sdk.h| 30 + src/parallels/parallels_utils.h | 3 + 7 files changed, 318 insertions(+), 9 deletions(-) create mode 100644 src/parallels/parallels_sdk.c create mode 100644 src/parallels/parallels_sdk.h -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/2] parallels: build with parallels SDK
Executing prlctl command is not an optimal way to interact with Parallels Cloud Server (PCS), it's better to use parallels SDK, which is a remote API to paralles dispatcher service. We prepared opensource version of this SDK and published it on github, it's distributed under LGPL license. Here is a git repo: https://github.com/Parallels/parallels-sdk. To build with parallels SDK user should get compiler and linker options from pkg-config 'parallels-sdk' file. So fix checks in configure script and build with parallels SDK, if that pkg-config file exists and add gcc options to makefile. Signed-off-by: Dmitry Guryanov dgurya...@parallels.com --- configure.ac| 11 +-- src/Makefile.am | 4 +++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/configure.ac b/configure.ac index f93c6c2..4318d24 100644 --- a/configure.ac +++ b/configure.ac @@ -1046,15 +1046,14 @@ dnl dnl Checks for the Parallels driver dnl +PKG_CHECK_MODULES([PARALLELS_SDK], [parallels-sdk], [PARALLELS_SDK_FOUND=yes], [PARALLELS_SDK_FOUND=no]) + if test $with_parallels = check; then -with_parallels=$with_linux -if test ! $host_cpu = 'x86_64'; then -with_parallels=no -fi +with_parallels=$PARALLELS_SDK_FOUND fi -if test $with_parallels = yes test $with_linux = no; then -AC_MSG_ERROR([The Parallels driver can be enabled on Linux only.]) +if test $with_parallels = yes test $PARALLELS_SDK_FOUND = no; then +AC_MSG_ERROR([Parallels Virtualization SDK is needed to build the Parallels driver.]) fi if test $with_parallels = yes; then diff --git a/src/Makefile.am b/src/Makefile.am index 538530e..dad7c7f 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1352,7 +1352,9 @@ if WITH_PARALLELS noinst_LTLIBRARIES += libvirt_driver_parallels.la libvirt_la_BUILT_LIBADD += libvirt_driver_parallels.la libvirt_driver_parallels_la_CFLAGS = \ - -I$(top_srcdir)/src/conf $(AM_CFLAGS) + -I$(top_srcdir)/src/conf $(AM_CFLAGS) \ + $(PARALLELS_SDK_CFLAGS) +libvirt_driver_parallels_la_LIBADD = $(PARALLELS_SDK_LIBS) libvirt_driver_parallels_la_SOURCES = $(PARALLELS_DRIVER_SOURCES) endif WITH_PARALLELS -- 1.9.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/2] parallels: login to parallels SDK
Add files parallels_sdk.c and parallels_sdk.h for code
which works with SDK, so libvirt's code will not mix with
dealing with parallels SDK.
To use Parallels SDK you must first call PrlApi_InitEx function,
and then you will be able to connect to a server with
PrlSrv_LoginLocalEx function. When you've done you must call
PrlApi_Deinit. So let's call PrlApi_InitEx on first .connectOpen,
count number of connections and deinitialize, when this counter
becomes zero.
Signed-off-by: Dmitry Guryanov dgurya...@parallels.com
---
Changes in v2:
* remove unneded parallelsStateDriver
* add missing parallels_sdk.c and parallels_sdk.h
po/POTFILES.in | 1 +
src/Makefile.am | 4 +-
src/parallels/parallels_driver.c | 40 ++-
src/parallels/parallels_sdk.c| 234 +++
src/parallels/parallels_sdk.h| 30 +
src/parallels/parallels_utils.h | 3 +
6 files changed, 310 insertions(+), 2 deletions(-)
create mode 100644 src/parallels/parallels_sdk.c
create mode 100644 src/parallels/parallels_sdk.h
diff --git a/po/POTFILES.in b/po/POTFILES.in
index f17b35f..4c1302d 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -96,6 +96,7 @@ src/openvz/openvz_driver.c
src/openvz/openvz_util.c
src/parallels/parallels_driver.c
src/parallels/parallels_network.c
+src/parallels/parallels_sdk.c
src/parallels/parallels_utils.c
src/parallels/parallels_utils.h
src/parallels/parallels_storage.c
diff --git a/src/Makefile.am b/src/Makefile.am
index dad7c7f..d4c6465 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -787,7 +787,9 @@ PARALLELS_DRIVER_SOURCES =
\
parallels/parallels_utils.c \
parallels/parallels_utils.h \
parallels/parallels_storage.c \
- parallels/parallels_network.c
+ parallels/parallels_network.c \
+ parallels/parallels_sdk.h \
+ parallels/parallels_sdk.c
BHYVE_DRIVER_SOURCES = \
bhyve/bhyve_capabilities.c \
diff --git a/src/parallels/parallels_driver.c b/src/parallels/parallels_driver.c
index 13a7d95..2431d00 100644
--- a/src/parallels/parallels_driver.c
+++ b/src/parallels/parallels_driver.c
@@ -55,6 +55,7 @@
#include parallels_driver.h
#include parallels_utils.h
+#include parallels_sdk.h
#define VIR_FROM_THIS VIR_FROM_PARALLELS
@@ -73,6 +74,9 @@ VIR_LOG_INIT(parallels.parallels_driver);
#define IS_CT(def) (STREQ_NULLABLE(def-os.type, exe))
+unsigned int numConns = 0;
+virMutex numConnsLock;
+
static int parallelsConnectClose(virConnectPtr conn);
static const char * parallelsGetDiskBusName(int bus) {
@@ -929,9 +933,25 @@ parallelsOpenDefault(virConnectPtr conn)
if (virMutexInit(privconn-lock) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(cannot initialize mutex));
-goto error;
+goto err_free;
+}
+
+virMutexLock(numConnsLock);
+numConns++;
+
+if (numConns == 1) {
+if (prlsdkInit()) {
+VIR_DEBUG(%s, _(Can't initialize Parallels SDK));
+virMutexUnlock(numConnsLock);
+goto err_free;
+}
}
+virMutexUnlock(numConnsLock);
+
+if (prlsdkConnect(privconn) 0)
+goto err_free;
+
if (!(privconn-caps = parallelsBuildCapabilities()))
goto error;
@@ -953,6 +973,9 @@ parallelsOpenDefault(virConnectPtr conn)
virObjectUnref(privconn-domains);
virObjectUnref(privconn-caps);
virStoragePoolObjListFree(privconn-pools);
+prlsdkDisconnect(privconn);
+prlsdkDeinit();
+ err_free:
VIR_FREE(privconn);
return VIR_DRV_OPEN_ERROR;
}
@@ -999,8 +1022,17 @@ parallelsConnectClose(virConnectPtr conn)
virObjectUnref(privconn-caps);
virObjectUnref(privconn-xmlopt);
virObjectUnref(privconn-domains);
+prlsdkDisconnect(privconn);
conn-privateData = NULL;
+virMutexLock(numConnsLock);
+numConns--;
+
+if (numConns == 0)
+prlsdkDeinit();
+
+virMutexUnlock(numConnsLock);
+
parallelsDriverUnlock(privconn);
virMutexDestroy(privconn-lock);
@@ -2483,6 +2515,12 @@ parallelsRegister(void)
VIR_FREE(prlctl_path);
+if (virMutexInit(numConnsLock) 0) {
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(cannot initialize mutex));
+return 0;
+}
+
if (virRegisterDriver(parallelsDriver) 0)
return -1;
if (parallelsStorageRegister())
diff --git a/src/parallels/parallels_sdk.c b/src/parallels/parallels_sdk.c
new file mode 100644
index 000..bedf32d
--- /dev/null
+++ b/src/parallels/parallels_sdk.c
@@ -0,0 +1,234 @@
+/*
+ * parallels_sdk.c: core driver functions for managing
+ * Parallels Cloud Server hosts
+ *
+ * Copyright (C) 2014
Re: [libvirt] [PATCH] blockcopy: allow block device destination
Ping
On 08/13/2014 02:00 PM, Eric Blake wrote:
To date, anyone performing a block copy and pivot ends up with
the destination being treated as disk type='file'. While this
works for data access for a block device, it has at least one
noticeable shortcoming: virDomainGetBlockInfo() reports allocation
differently for block devices visited as files (the size of the
device) than for block devices visited as disk type='block'
(the maximum sector used, as reported by qemu); and this difference
is significant when trying to manage qcow2 format on block devices
that can be grown as needed.
I still plan to add a virDomainBlockCopy() API that takes the
destination disk as XML, allowing full expressive capability
to copy to a network disk. But a new API can't be backported,
while a new flag to an existing API can. So this patch enhances
blockcopy to let the user flag when the resulting XML after the
copy must list the device as type='block'.
* include/libvirt/libvirt.h.in (VIR_DOMAIN_BLOCK_REBASE_COPY_DEV):
New flag.
* src/libvirt.c (virDomainBlockRebase): Document it.
* tools/virsh-domain.c (opts_block_copy, blockJobImpl): Add
--blockdev option.
* tools/virsh.pod (blockcopy): Document it.
* src/qemu/qemu_driver.c (qemuDomainBlockRebase): Allow new flag.
(qemuDomainBlockCopy): Remember the flag.
* tests/qemuxml2argvdata/qemuxml2argv-disk-mirror.xml: Test it.
Signed-off-by: Eric Blake ebl...@redhat.com
---
include/libvirt/libvirt.h.in |2 ++
src/libvirt.c |8 ++--
src/qemu/qemu_driver.c | 12
.../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml |4 ++--
tools/virsh-domain.c |6 ++
tools/virsh.pod|7 +--
6 files changed, 29 insertions(+), 10 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index 47ea695..f2a02ea 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -2590,6 +2590,8 @@ typedef enum {
VIR_DOMAIN_BLOCK_REBASE_RELATIVE = 1 4, /* Keep backing chain
referenced using relative
names */
+VIR_DOMAIN_BLOCK_REBASE_COPY_DEV = 1 5, /* Treat destination as block
+ device instead of file */
} virDomainBlockRebaseFlags;
int virDomainBlockRebase(virDomainPtr dom, const char *disk,
diff --git a/src/libvirt.c b/src/libvirt.c
index 992e4f2..c4643e8 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -19881,7 +19881,10 @@ virDomainBlockPull(virDomainPtr dom, const char
*disk,
* pre-create files with relative backing file names, rather than the default
* of absolute backing file names; as a security precaution, you should
* generally only use reuse_ext with the shallow flag and a non-raw
- * destination file.
+ * destination file. By default, the copy destination will be treated as
+ * type='file', but using VIR_DOMAIN_BLOCK_REBASE_COPY_DEV treats the
+ * destination as type='block' (affecting how virDomainGetBlockInfo() will
+ * report allocation after pivoting).
*
* A copy job has two parts; in the first phase, the @bandwidth parameter
* affects how fast the source is pulled into the destination, and the job
@@ -19950,7 +19953,8 @@ virDomainBlockRebase(virDomainPtr dom, const char
*disk,
virCheckNonNullArgGoto(base, error);
} else if (flags (VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
-VIR_DOMAIN_BLOCK_REBASE_COPY_RAW)) {
+VIR_DOMAIN_BLOCK_REBASE_COPY_RAW |
+VIR_DOMAIN_BLOCK_REBASE_COPY_DEV)) {
virReportInvalidArg(flags,
_(use of flags in %s requires a copy job),
__FUNCTION__);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b6219ba..e74227e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15295,7 +15295,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
/* Preliminaries: find the disk we are editing, sanity checks */
virCheckFlags(VIR_DOMAIN_BLOCK_REBASE_SHALLOW |
- VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT, -1);
+ VIR_DOMAIN_BLOCK_REBASE_REUSE_EXT |
+ VIR_DOMAIN_BLOCK_REBASE_COPY_DEV, -1);
priv = vm-privateData;
cfg = virQEMUDriverGetConfig(driver);
@@ -15374,7 +15375,8 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
if (VIR_ALLOC(mirror) 0)
goto endjob;
/* XXX Allow non-file mirror destinations */
-mirror-type = VIR_STORAGE_TYPE_FILE;
+mirror-type = flags VIR_DOMAIN_BLOCK_REBASE_COPY_DEV ?
+VIR_STORAGE_TYPE_BLOCK : VIR_STORAGE_TYPE_FILE;
Re: [libvirt] [PATCH] qemu: check for active domain after agent interaction
On 08/21/2014 12:50 AM, Martin Kletzander wrote: On Wed, Aug 20, 2014 at 03:35:08PM -0600, Eric Blake wrote: Commit b606bbb41 reminded me that any time we drop locks to run back-to-back guest interaction commands, we have to check that the guest didn't disappear in between the two commands. A quick audit found a couple of spots that were missing this check. * src/qemu/qemu_driver.c (qemuDomainShutdownFlags) (qemuDomainSetVcpusFlags): Check that domain is still up. Signed-off-by: Eric Blake ebl...@redhat.com --- I'm thinking about a place where we could stick this to make it more generic, but can't find one. That would be a completely different cleanup anyway, so ACK to this. Thanks; pushed. About the only idea I had would be making qemuDomainObjExitAgent() return a value marked ATTRIBUTE_RETURN_CHECK, where the caller can be informed if the guest is still running every time it regains control after temporarily dropping locks; but that is, as you say, a completely different cleanup because it would touch a lot of callers. -- 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] [PATCH v2] spec: drop anything older than Fedora 13
RHEL 5 is based on libvirt 0.8.2, as was Fedora 13. RHEL 5 also
happens to be the oldest box that we actively support with a
buildbot, so it is time to clean up some crufty conditionals in
the spec file that no longer are necessary for modern Fedora.
Although it is probably okay to make further simplifications to
a newer minimum Fedora version, that can be done as a later patch.
This patch just focuses on cleaning any comparison of %{?fedora}
that will always be true or false once we assume a minimum of F13.
* libvirt.spec.in: Make with_audit default to on. Move other
conditionals to a single RHEL-5 block. Simplify any fedora
comparison older than 13. Document our assumptions.
Signed-off-by: Eric Blake ebl...@redhat.com
---
v2: rebase to latest libvirt.git
libvirt.spec.in | 69 -
1 file changed, 24 insertions(+), 45 deletions(-)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 3932313..9126277 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1,5 +1,7 @@
# -*- rpm-spec -*-
+# This spec file assumes you are building for Fedora 13 or newer,
+# or for RHEL 5 or newer. It may need some tweaks for other distros.
# If neither fedora nor rhel was defined, try to guess them from %{dist}
%if !0%{?rhel} !0%{?fedora}
%{expand:%(echo %{?dist} | \
@@ -139,7 +141,6 @@
%define with_libpcap 0%{!?_without_libpcap:0}
%define with_macvtap 0%{!?_without_macvtap:0}
%define with_libnl 0%{!?_without_libnl:0}
-%define with_audit 0%{!?_without_audit:0}
%define with_dtrace0%{!?_without_dtrace:0}
%define with_cgconfig 0%{!?_without_cgconfig:0}
%define with_sanlock 0%{!?_without_sanlock:0}
@@ -153,6 +154,7 @@
# Non-server/HV driver defaults which are always enabled
%define with_sasl 0%{!?_without_sasl:1}
+%define with_audit 0%{!?_without_audit:1}
# Finally set the OS / architecture specific special cases
@@ -223,31 +225,21 @@
%define with_libxl 0
%endif
-# PolicyKit was introduced in Fedora 8 / RHEL-6 or newer
-%if 0%{?fedora} = 8 || 0%{?rhel} = 6
-%define with_polkit0%{!?_without_polkit:1}
-%endif
-
-# libcapng is used to manage capabilities in Fedora 12 / RHEL-6 or newer
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
-%define with_capng 0%{!?_without_capng:1}
-%endif
-
# fuse is used to provide virtualized /proc for LXC
%if 0%{?fedora} = 17 || 0%{?rhel} = 7
%define with_fuse 0%{!?_without_fuse:1}
%endif
-# netcf is used to manage network interfaces in Fedora 12 / RHEL-6 or newer
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
-%define with_netcf 0%{!?_without_netcf:%{server_drivers}}
-%endif
-
-# udev is used to manage host devices in Fedora 12 / RHEL-6 or newer
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
-%define with_udev 0%{!?_without_udev:%{server_drivers}}
-%else
+# RHEL 5 lacks newer tools
+%if 0%{?rhel} == 5
%define with_hal 0%{!?_without_hal:%{server_drivers}}
+%else
+%define with_polkit0%{!?_without_polkit:1}
+%define with_capng 0%{!?_without_capng:1}
+%define with_netcf 0%{!?_without_netcf:%{server_drivers}}
+%define with_udev 0%{!?_without_udev:%{server_drivers}}
+%define with_yajl 0%{!?_without_yajl:%{server_drivers}}
+%define with_dtrace 1
%endif
# interface requires netcf
@@ -255,11 +247,6 @@
%define with_interface 0
%endif
-# Enable yajl library for JSON mode with QEMU
-%if 0%{?fedora} = 13 || 0%{?rhel} = 6
-%define with_yajl 0%{!?_without_yajl:%{server_drivers}}
-%endif
-
# Enable sanlock library for lock management with QEMU
# Sanlock is available only on arches where kvm is available for RHEL
%if 0%{?fedora} = 16
@@ -320,16 +307,8 @@
%define with_libnl 1
%endif
-%if 0%{?fedora} = 11 || 0%{?rhel} = 5
-%define with_audit0%{!?_without_audit:1}
-%endif
-
-%if 0%{?fedora} = 13 || 0%{?rhel} = 6
-%define with_dtrace 1
-%endif
-
# Pull in cgroups config system
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} || 0%{?rhel} = 6
%if %{with_qemu} || %{with_lxc}
%define with_cgconfig 0%{!?_without_cgconfig:1}
%endif
@@ -349,7 +328,7 @@
# Force QEMU to run as non-root
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} || 0%{?rhel} = 6
%define qemu_user qemu
%define qemu_group qemu
%else
@@ -473,7 +452,7 @@ BuildRequires: libattr-devel
# For pool-build probing for existing pools
BuildRequires: libblkid-devel = 2.17
%endif
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} || 0%{?rhel} = 6
# for augparse, optionally used in testing
BuildRequires: augeas
%endif
@@ -538,7 +517,7 @@ BuildRequires: cyrus-sasl-devel
%if 0%{?fedora} = 20 || 0%{?rhel} = 7
BuildRequires: polkit-devel = 0.112
%else
-%if 0%{?fedora} = 12 || 0%{?rhel} = 6
+%if 0%{?fedora} || 0%{?rhel} = 6
BuildRequires: polkit-devel = 0.93
%else
BuildRequires: PolicyKit-devel = 0.6
@@ -621,7
