Dear list,
there's been a lot of development in QEMU on this part. And I think it's
settled down enough long so I can start looking at it. So I'd like to
hear you opinion what's the best way to expose this in libvirt.
OVMF can bee looked at as a UEFI enablement in guest. Standard UEFI
consists of two parts:
a) the firmware binary image (RO)
b) UEFI variables flash (RW)
IIUC both of these are to be passed to qemu on the command line as:
-drive file=img_1,if=pflash,format=raw,readonly \
-drive file=img_2,if=pflash,format=raw
Subsequently, -bios parameter should be dropped. The idea of splitting
the UEFI into two files allows distros to update the UEFI firmware (FW
for short) without modifying guest written UEFI variables file (the
variables should have unified name so they should be transferable
between two versions of UEFI FW).
So my question is: how to expose this in the domain XML? We have the
element which handles the booting arguments. It can have
(which would be great for the FW, wouldn't it?). But then we need to
invent a different element (say ) which would contain path the
the UEFI vars file. Moreover, the element would exclude other elements
like , or . So my proposal is:
hvm
/path/to/uefi.fw
/path/to/uefi.nvvarstore
Does this make any sense or am I just blabbing?
Michal
BTW OVMF stands for Open Virtual Machine Firmware.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11.07.2014 18:59, Eric Blake wrote:
> On 07/11/2014 03:32 AM, Michal Privoznik wrote:
>> There's this trend in libvirt of using enum types wherever possible.
>> Now that I'm at virSecurityLabelDef let's rework @type item of the
>> structure so we don't have to typecast it elsewhere.
>>
>> Signed-off-by: Michal Privoznik
>> ---
>> src/conf/domain_conf.c | 6 --
>> src/security/security_dac.c | 2 +-
>> src/util/virseclabel.h | 2 +-
>> 3 files changed, 6 insertions(+), 4 deletions(-)
>
> I'm not quite as sure about this one. This solves the issue of how to
> detect errors when parsing strings to enum, but required the use of an
> intermediate variable which in turn made the patch a net gain in lines
> of code. If someone forgets to use the intermediate variable for
> parsing, this backfires. On the other hand, parsing string to enum
> should be done in just one location, and that's the location touched by
> this patch. I'm 50-50 on whether to take this, so I'd like someone else
> to chime in with an opinion.
I hear you. This patch is just a refactor. It does not add anything useful nor
solve any issue. It's okay if dropped. But the more I think about our
vir*TypeFromString() the more I feel we should do something about it. How about
making it follow our typical function return pattern:
int func() {
virMyFavourite x;
const char *string;
if (virMyFavouriteTypeFromString(string, &x) < 0) {
virReportError("unknown value: %s", string);
goto error;
}
That is, we need this diff:
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 95d1ff9..40075e9 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -407,15 +407,20 @@ virParseVersionString(const char *str, unsigned long
*version,
int virEnumFromString(const char *const*types,
unsigned int ntypes,
- const char *type)
+ const char *type,
+ int *val)
{
size_t i;
if (!type)
return -1;
-for (i = 0; i < ntypes; i++)
-if (STREQ(types[i], type))
-return i;
+for (i = 0; i < ntypes; i++) {
+if (STREQ(types[i], type)) {
+if (val)
+*val = i;
+return 0;
+}
+}
return -1;
}
diff --git a/src/util/virutil.h b/src/util/virutil.h
index 2bb74e2..670b2e1 100644
--- a/src/util/virutil.h
+++ b/src/util/virutil.h
@@ -73,7 +73,8 @@ char *virIndexToDiskName(int idx, const char *prefix);
int virEnumFromString(const char *const*types,
unsigned int ntypes,
- const char *type);
+ const char *type,
+ int *val);
const char *virEnumToString(const char *const*types,
unsigned int ntypes,
@@ -87,10 +88,10 @@ const char *virEnumToString(const char *const*types,
ARRAY_CARDINALITY(name ## TypeList), \
type); \
} \
-int name ## TypeFromString(const char *type) { \
+int name ## TypeFromString(const char *type, name *val) { \
return virEnumFromString(name ## TypeList, \
ARRAY_CARDINALITY(name ## TypeList), \
- type); \
+ type, (int *) val);\
}
# define VIR_ENUM_DECL(name) \
And then a tons of follow up patches. Or even make virEnumString() report the
error (that could save a lot of virReportError() calls).
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 12.07.2014 06:47, Lichunhe wrote:
If the ovs service stop abnormal, or host cold reboot, vm is destroyed after
ovs service stop. The ovs port which connect to interface of vm will not be
clear. When the ovs service restart, recover configuration from db, but the
interface is no exist, port recovery failed, and then vm restart on the same
host, libvirt add port again, but the port configuration is same as before, ovs
will not connect the interface, only store the configuration in db. Below will
trigger this problem,
We like the commit messages wrapped at 80 characters per line.
Moreover, I find it somehow hard to parse.
virsh start vm
service openvswitch-switch stop
virsh destroy vm
service openvswitch-switch start
virsh start vm
Signed-off-by: Chunhe Li
---
src/util/virnetdevopenvswitch.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
I had some difficulties applying this patch. Did you hand edit it before
sending?
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index 9bcbfb1..2c414ad 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -84,8 +84,8 @@ int virNetDevOpenvswitchAddPort(const char *brname, const
char *ifname,
cmd = virCommandNew(OVSVSCTL);
-virCommandAddArgList(cmd, "--timeout=5", "--", "--may-exist", "add-port",
-brname, ifname, NULL);
+virCommandAddArgList(cmd, "--timeout=5", "--", "--if-exists", "del-port",
Trailing whitespace.
+ifname, "--", "add-port", brname, ifname, NULL);
if (virtVlan && virtVlan->nTags > 0) {
--
1.9.2.msysgit.0
Strange git version string :)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Well, I've pointed out enough problems to request v3, but since this
practically oneliner, I rather fix all the nits and push.
ACKed and pushed.
Congratulations on your first libvirt contribution!
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 06.07.2014 18:28, Roman Bogorodskiy wrote:
This series implements support for querying network interface
stats on (Free)BSD.
It's more of an RFC, because I'm uncertain about few things:
- It feels a little strange to have a source file that implements
only a single function like this. I am wondering if it would be better
to just move it to something like util/virnetdev.c?
- FreeBSD stores interface data in the if_data struct and a number of outgoing
packet drops is stored in a field 'ifi_oqdrops'. This field was added
in -CURRENT and later merged back to 10-STABLE. In order not to break
the ABI, it's available only if _IFI_OQDROPS is defined. I've added
a configure.ac check which adds -D_IFI_OQDROPS before checking this field
and resetting it back if it is not present. This way, this flag will
present when the field is available even if the flag is not needed
(e.g. on -CURRENT). Is there a better way of doing it? I was thinking
about trying to check this field without the flag and if it fails check
one more time with the flag, but it looks a little messy.
- Did I get it right that the stats reported are from the guest POV, e.g.
when downloading a large file from guest, it should look like:
vnet0 rx_bytes 731603341
vnet0 rx_packets 518354
vnet0 rx_errs 0
vnet0 rx_drop 0
vnet0 tx_bytes 17577834
vnet0 tx_packets 264226
vnet0 tx_errs 0
vnet0 tx_drop 0
Roman Bogorodskiy (2):
util: virstatslinux: make more generic
Implement interface stats for BSD
configure.ac | 13 -
po/POTFILES.in | 2 +-
src/Makefile.am | 2 +-
src/libvirt_linux.syms | 3 --
src/libvirt_private.syms | 2 +
src/lxc/lxc_driver.c | 2 +-
src/openvz/openvz_driver.c | 2 +-
src/qemu/qemu_driver.c | 16 +-
src/uml/uml_driver.c | 2 +-
src/util/{virstatslinux.c => virstats.c} | 93 +---
src/util/{virstatslinux.h => virstats.h} | 12 ++---
src/xen/xen_hypervisor.c | 2 +-
tests/statstest.c| 2 +-
13 files changed, 102 insertions(+), 51 deletions(-)
rename src/util/{virstatslinux.c => virstats.c} (61%)
rename src/util/{virstatslinux.h => virstats.h} (77%)
ACK to both patches.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 15.07.2014 08:33, Martin Kletzander wrote:
On Fri, Jul 11, 2014 at 05:11:00PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
There were numerous places where numatune configuration (and thus
domain config as well) was changed in different ways. On some
places this even resulted in persistent domain definition not to be
stable (it would change with daemon's restart).
In order to uniformly change how numatune config is dealt with, all
the internals are now accessible directly only in numatune_conf.c and
outside this file accessors must be used.
Signed-off-by: Martin Kletzander
---
po/POTFILES.in | 1 +
src/conf/domain_conf.c | 159 ++-
src/conf/domain_conf.h | 8 +-
src/conf/numatune_conf.c | 316
+
src/conf/numatune_conf.h | 72 -
src/libvirt_private.syms | 11 +
src/lxc/lxc_cgroup.c | 19 +-
src/lxc/lxc_controller.c | 5 +-
src/lxc/lxc_native.c | 15 +-
src/parallels/parallels_driver.c | 7 +-
src/qemu/qemu_cgroup.c | 23 +-
src/qemu/qemu_driver.c | 84 +++---
src/qemu/qemu_process.c| 8 +-
src/util/virnuma.c | 48 ++--
src/util/virnuma.h | 2 +-
.../qemuxml2argv-numatune-auto-prefer.xml | 29 ++
.../qemuxml2xmlout-numatune-auto-prefer.xml| 29 ++
tests/qemuxml2xmltest.c| 2 +
18 files changed, 553 insertions(+), 285 deletions(-)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-numatune-auto-prefer.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-numatune-auto-prefer.xml
Nice.
Thanks :)
[...]
+tmp = virXMLPropString(node, "nodeset");
+if (tmp && virBitmapParse(tmp, 0, &nodeset,
VIR_DOMAIN_CPUMASK_LEN) < 0)
+goto cleanup;
+VIR_FREE(tmp);
+
+if (virDomainNumatuneSet(def, placement, mode, nodeset) < 0)
The virDomainNumatuneSet() takes a copy of @nodeset, so you need to call
virBitmaskFree(nodeset); at the cleanup label.
Yep, that happens when you change the behaviour of a function that
used to steal a pointer, in a rebase. Thanks!
+goto cleanup;
+
+if (!n) {
+ret = 0;
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(tmp);
+return ret;
+}
+
+int
+virDomainNumatuneFormatXML(virBufferPtr buf,
+ virDomainNumatunePtr numatune)
+{
+const char *tmp = NULL;
s /const// ..
+
+if (!numatune)
+return 0;
+
+virBufferAddLit(buf, "\n");
+virBufferAdjustIndent(buf, 2);
+
+tmp = virDomainNumatuneMemModeTypeToString(numatune->memory.mode);
+virBufferAsprintf(buf, "memory.placement ==
VIR_DOMAIN_NUMATUNE_PLACEMENT_STATIC) {
+if (!(tmp = virBitmapFormat(numatune->memory.nodeset)))
+return -1;
+virBufferAsprintf(buf, "nodeset='%s'/>\n", tmp);
+VIR_FREE(tmp);
.. because free()-ing a const char * is not nice. If you, however, do
this I bet you'll get error in TypeToString(). So just leave tmp as
const char * and introduce char *nodeset;
I take the 'const' as a sign of the fact that I won't be modifying
any part of the string. Just adding 'const' to a pointer should be
perfectly OK, but I have not objections to your idea, so I squashed
this in:
Well, I look at free()-ing as modification of the pointee. Therefore
freeing a const pointer is in fact its modification and hence should be
rejected. It's just that our VIR_FREE throws away the const-ness of
passed pointers. Maybe (as completely separate patchset) we may fix the
VIR_FREE() macro which is obviously const-incorrect.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 15.07.2014 08:23, Martin Kletzander wrote:
On Fri, Jul 11, 2014 at 05:11:05PM +0200, Michal Privoznik wrote:
On 08.07.2014 13:50, Martin Kletzander wrote:
In XML format, by definition, order of fields should not matter, so
order of parsing the elements doesn't affect the end result. When
specifying guest NUMA cells, we depend only on the order of the 'cell'
elements. With this patch all older domain XMLs are parsed as before,
but with the 'id' attribute they are parsed and formatted according to
that field. This will be useful when we have tuning settings for
particular guest NUMA node.
Signed-off-by: Martin Kletzander
---
docs/formatdomain.html.in | 11 +++---
docs/schemas/domaincommon.rng | 5 +++
src/conf/cpu_conf.c| 39
+++---
src/conf/cpu_conf.h| 3 +-
src/qemu/qemu_command.c| 2 +-
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa1.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa2.xml | 6 ++--
tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml | 25 ++
tests/qemuxml2argvtest.c | 1 +
.../qemuxml2xmlout-cpu-numa1.xml | 28
.../qemuxml2xmlout-cpu-numa2.xml | 28
tests/qemuxml2xmltest.c| 3 ++
12 files changed, 139 insertions(+), 18 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-cpu-numa3.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa1.xml
create mode 100644
tests/qemuxml2xmloutdata/qemuxml2xmlout-cpu-numa2.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index b69da4c..ad87b7c 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
[...]
@@ -1041,8 +1041,11 @@
Each cell element specifies a NUMA cell or a
NUMA node.
cpus specifies the CPU or range of CPUs that are
part of
the node. memory specifies the node memory in
kibibytes
- (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
- or nodeid in the increasing order starting from 0.
+ (i.e. blocks of 1024 bytes). All cells should have
id
+ attribute in case referring to some cell is necessary in the
code,
+ otherwise the cells are assigned ids in the increasing order
starting
+ from 0. Mixing cells with and without the id
attribute
+ is not recommended as it may result in unwanted behaviour.
I'd note here, that the @id attribute is since 1.2.7
I wasn't sure this is needed for attributes, but it cannot hurt,
right? The new hunk would now look like this:
@@ -1039,10 +1039,15 @@
Each cell element specifies a NUMA cell or a NUMA node.
- cpus specifies the CPU or range of CPUs that are
part of
- the node. memory specifies the node memory in kibibytes
- (i.e. blocks of 1024 bytes). Each cell or node is assigned cellid
- or nodeid in the increasing order starting from 0.
+ cpus specifies the CPU or range of CPUs that are
+ part of the node. memory specifies the node memory
+ in kibibytes (i.e. blocks of 1024 bytes).
+ Since 1.2.7 all cells should
+ have id attribute in case referring to some cell is
+ necessary in the code, otherwise the cells are
+ assigned ids in the increasing order starting from
+ 0. Mixing cells with and without the id attribute
+ is not recommended as it may result in unwanted behaviour.
--
Is that OK?
Okay.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Since we've corrected all the callers that passed a const pointer to
VIR_FREE() we may drop the typecasting horribility and let the macro
be a simple wrapper over the virFree() function.
Signed-off-by: Michal Privoznik
---
src/util/viralloc.h | 15 +--
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 7125e67..71b4a45 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -547,20 +547,7 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
*
* This macro is safe to use on arguments with side effects.
*/
-# if !STATIC_ANALYSIS
-/* The ternary ensures that ptr is a pointer and not an integer type,
- * while evaluating ptr only once. This gives us extra compiler
- * safety when compiling under gcc. For now, we intentionally cast
- * away const, since a number of callers safely pass const char *.
- */
-# define VIR_FREE(ptr) virFree((void *) (1 ? (const void *) &(ptr) : (ptr)))
-# else
-/* The Coverity static analyzer considers the else path of the "?:" and
- * flags the VIR_FREE() of the address of the address of memory as a
- * RESOURCE_LEAK resulting in numerous false positives (eg, VIR_FREE(&ptr))
- */
-# define VIR_FREE(ptr) virFree((void *) &(ptr))
-# endif
+# define VIR_FREE(ptr) virFree(&(ptr))
void virAllocTestInit(void);
int virAllocTestCount(void);
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Up to now it's possible to do something like this:
const char *ptr;
ptr = strdup("my example string");
VIR_FREE(ptr);
The problem is, const char * pointers should not be modified (and
freeing them is kind of modification). We should avoid this. A little
trick is used: assigning a const pointer into 'void *' triggers
compiler warning about discarding 'const' qualifier from pointer. So
the virFree() function gains new dummy argument, that is not touched
anyhow, just fulfills the const correctness check duty.
Signed-off-by: Michal Privoznik
---
src/util/viralloc.c | 6 --
src/util/viralloc.h | 20
src/xenapi/xenapi_utils.c | 2 +-
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/util/viralloc.c b/src/util/viralloc.c
index be9f0fe..0134e67 100644
--- a/src/util/viralloc.c
+++ b/src/util/viralloc.c
@@ -372,7 +372,7 @@ void virShrinkN(void *ptrptr, size_t size, size_t
*countptr, size_t toremove)
ignore_value(virReallocN(ptrptr, size, *countptr -= toremove,
false, 0, NULL, NULL, 0));
else {
-virFree(ptrptr);
+virFree(NULL, ptrptr);
*countptr = 0;
}
}
@@ -569,13 +569,15 @@ int virAllocVar(void *ptrptr,
/**
* virFree:
+ * @ptr: dummy pointer to check const correctness
* @ptrptr: pointer to pointer for address of memory to be freed
*
* Release the chunk of memory in the pointer pointed to by
* the 'ptrptr' variable. After release, 'ptrptr' will be
* updated to point to NULL.
*/
-void virFree(void *ptrptr)
+void virFree(void *ptr ATTRIBUTE_UNUSED,
+ void *ptrptr)
{
int save_errno = errno;
diff --git a/src/util/viralloc.h b/src/util/viralloc.h
index 71b4a45..8fbe56f 100644
--- a/src/util/viralloc.h
+++ b/src/util/viralloc.h
@@ -76,7 +76,7 @@ int virAllocVar(void *ptrptr, size_t struct_size, size_t
element_size, size_t co
bool report, int domcode, const char *filename,
const char *funcname, size_t linenr)
ATTRIBUTE_RETURN_CHECK ATTRIBUTE_NONNULL(1);
-void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
+void virFree(void *ptr, void *ptrptr) ATTRIBUTE_NONNULL(2);
/**
* VIR_ALLOC:
@@ -543,11 +543,23 @@ void virFree(void *ptrptr) ATTRIBUTE_NONNULL(1);
* @ptr: pointer holding address to be freed
*
* Free the memory stored in 'ptr' and update to point
- * to NULL.
+ * to NULL. Moreover, this macro has a side effect in
+ * form of evaluating passed argument multiple times.
+ * But advantage is, it checks the cont correctness
+ * (that you are not freeing a const pointer).
+ */
+# define VIR_FREE(ptr) virFree(ptr, &(ptr))
+
+/**
+ * VIR_FREE_BROKEN:
+ * @ptr: pointer holding address to be freed
*
- * This macro is safe to use on arguments with side effects.
+ * Twin macro of VIR_FREE. While it does evaluate
+ * argument only once, it does not check const
+ * correctness and therefore you want to use it if and
+ * only if necessary.
*/
-# define VIR_FREE(ptr) virFree(&(ptr))
+# define VIR_FREE_BROKEN(ptr) virFree(NULL, &(ptr))
void virAllocTestInit(void);
int virAllocTestCount(void);
diff --git a/src/xenapi/xenapi_utils.c b/src/xenapi/xenapi_utils.c
index a80d136..db555d2 100644
--- a/src/xenapi/xenapi_utils.c
+++ b/src/xenapi/xenapi_utils.c
@@ -50,7 +50,7 @@ xenSessionFree(xen_session *session)
VIR_FREE(session->error_description);
}
/* The session_id member is type of 'const char *'. Sigh. */
-VIR_FREE(session->session_id);
+VIR_FREE_BROKEN(session->session_id);
VIR_FREE(session);
}
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
There are just two places where we rely on the fact that VIR_FREE()
macro is without any side effects. In the future, this property of the
macro is going to change, so we need the code to be adjusted to deal
with argument passed to VIR_FREE() being evaluated multiple times.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index ce4d4d8..d60a60e 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -180,8 +180,10 @@ virNetworkDNSTxtDefClear(virNetworkDNSTxtDefPtr def)
static void
virNetworkDNSHostDefClear(virNetworkDNSHostDefPtr def)
{
-while (def->nnames)
-VIR_FREE(def->names[--def->nnames]);
+size_t i;
+
+for (i = 0; i < def->nnames; i++)
+VIR_FREE(def->names[i]);
VIR_FREE(def->names);
}
@@ -197,9 +199,11 @@ virNetworkDNSSrvDefClear(virNetworkDNSSrvDefPtr def)
static void
virNetworkDNSDefClear(virNetworkDNSDefPtr def)
{
+size_t i;
+
if (def->forwarders) {
-while (def->nfwds)
-VIR_FREE(def->forwarders[--def->nfwds]);
+for (i = 0; i < def->nfwds; i++)
+VIR_FREE(def->forwarders[i]);
VIR_FREE(def->forwarders);
}
if (def->txts) {
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 15.07.2014 15:27, Martin Kletzander wrote:
On Tue, Jul 15, 2014 at 02:38:36PM +0200, Michal Privoznik wrote:
Up to now it's possible to do something like this:
const char *ptr;
ptr = strdup("my example string");
VIR_FREE(ptr);
The problem is, const char * pointers should not be modified (and
freeing them is kind of modification). We should avoid this. A little
trick is used: assigning a const pointer into 'void *' triggers
compiler warning about discarding 'const' qualifier from pointer. So
the virFree() function gains new dummy argument, that is not touched
anyhow, just fulfills the const correctness check duty.
Signed-off-by: Michal Privoznik
---
src/util/viralloc.c | 6 --
src/util/viralloc.h | 20
src/xenapi/xenapi_utils.c | 2 +-
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/src/util/viralloc.c b/src/util/viralloc.c
index be9f0fe..0134e67 100644
--- a/src/util/viralloc.c
+++ b/src/util/viralloc.c
[...]
@@ -569,13 +569,15 @@ int virAllocVar(void *ptrptr,
/**
* virFree:
+ * @ptr: dummy pointer to check const correctness
* @ptrptr: pointer to pointer for address of memory to be freed
*
* Release the chunk of memory in the pointer pointed to by
* the 'ptrptr' variable. After release, 'ptrptr' will be
* updated to point to NULL.
*/
-void virFree(void *ptrptr)
+void virFree(void *ptr ATTRIBUTE_UNUSED,
+ void *ptrptr)
What if you don't add another argument, but just change the void
*ptrptr to void **ptrptr. Compiler shouldn't be mad about not knowing
the size resulting of de-referencing ptrptr, you get the check you
want and keep the macro without side-effects.
That won't work. Consider:
char *tmp;
VIR_FREE(tmp);
which in turn is equal to:
virFree(&tmp);
so the &tmp is type of 'char **' while virFree() would expect 'void **'
which confuses compiler.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 15.07.2014 14:58, Ján Tomko wrote:
On 07/15/2014 02:38 PM, Michal Privoznik wrote:
There are just two places where we rely on the fact that VIR_FREE()
macro is without any side effects. In the future, this property of the
macro is going to change, so we need the code to be adjusted to deal
with argument passed to VIR_FREE() being evaluated multiple times.
Signed-off-by: Michal Privoznik
---
src/conf/network_conf.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
NACK, this completely removes the side effect. You need to adjust the callers
for that.
Well, the arrays that n refer to are freed immediately, but I
agree that we should keep the code clean.
IMO we should set n to 0 in all *Clear functions, not just
virNetworkDNSHostDefClear, after which NULL might be dereferenced based on the
n variable.
I don't see how that could be possible with current code. The
virNetworkDNSHostDefClear is called only on cleanup paths where
accessing def->names or def->forwarders is not possible anymore.
Jan
Okay, consider this squashed in:
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index d60a60e..dcb521f 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -184,6 +184,7 @@ virNetworkDNSHostDefClear(virNetworkDNSHostDefPtr def)
for (i = 0; i < def->nnames; i++)
VIR_FREE(def->names[i]);
+def->nnames = 0;
VIR_FREE(def->names);
}
@@ -204,6 +205,7 @@ virNetworkDNSDefClear(virNetworkDNSDefPtr def)
if (def->forwarders) {
for (i = 0; i < def->nfwds; i++)
VIR_FREE(def->forwarders[i]);
+def->nfwds = 0;
VIR_FREE(def->forwarders);
}
if (def->txts) {
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
There's this question on the list that is asked over and over again.
How do I get {cpu, memory, ...} usage in percentage? Or its modified
version: How do I plot nice graphs like virt-manager does?
It would be nice if we have an example to inspire people. And that's
what domtop should do. Yes, it could be written in different ways, but
I've chosen this one as I think it show explicitly what users need to
implement in order to imitate virt-manager's graphing.
Signed-off-by: Michal Privoznik
---
.gitignore | 1 +
Makefile.am | 2 +-
cfg.mk | 2 +-
configure.ac| 1 +
examples/domtop/Makefile.am | 27 +++
examples/domtop/domtop.c| 388
libvirt.spec.in | 2 +-
7 files changed, 420 insertions(+), 3 deletions(-)
create mode 100644 examples/domtop/Makefile.am
create mode 100644 examples/domtop/domtop.c
diff --git a/.gitignore b/.gitignore
index 2d4d401..90fee91 100644
--- a/.gitignore
+++ b/.gitignore
@@ -75,6 +75,7 @@
/examples/dominfo/info1
/examples/domsuspend/suspend
/examples/dommigrate/dommigrate
+/examples/domtop/domtop
/examples/hellolibvirt/hellolibvirt
/examples/openauth/openauth
/gnulib/lib/*
diff --git a/Makefile.am b/Makefile.am
index a374e1a..4aafe94 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -24,7 +24,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs
gnulib/tests \
examples/dominfo examples/domsuspend examples/apparmor \
examples/xml/nwfilter examples/openauth examples/systemtap \
tools/wireshark examples/dommigrate \
- examples/lxcconvert
+ examples/lxcconvert examples/domtop
ACLOCAL_AMFLAGS = -I m4
diff --git a/cfg.mk b/cfg.mk
index baaab71..9880704 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1078,7 +1078,7 @@ exclude_file_name_regexp--sc_prohibit_sprintf = \
exclude_file_name_regexp--sc_prohibit_strncpy = ^src/util/virstring\.c$$
exclude_file_name_regexp--sc_prohibit_strtol = \
-
^(src/(util/virsexpr|(vbox|xen|xenxs)/.*)\.c)|(examples/domsuspend/suspend.c)$$
+
^(src/(util/virsexpr|(vbox|xen|xenxs)/.*)\.c)|(examples/domsuspend/suspend.c)|(examples/domtop/domtop.c)$$
exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/virxml\.c$$
diff --git a/configure.ac b/configure.ac
index 8001e24..f37c716 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2755,6 +2755,7 @@ AC_CONFIG_FILES([\
examples/domsuspend/Makefile \
examples/dominfo/Makefile \
examples/dommigrate/Makefile \
+examples/domtop/Makefile \
examples/openauth/Makefile \
examples/hellolibvirt/Makefile \
examples/systemtap/Makefile \
diff --git a/examples/domtop/Makefile.am b/examples/domtop/Makefile.am
new file mode 100644
index 000..c5cb6c7
--- /dev/null
+++ b/examples/domtop/Makefile.am
@@ -0,0 +1,27 @@
+## Process this file with automake to produce Makefile.in
+
+## Copyright (C) 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
+## License as published by the Free Software Foundation; either
+## version 2.1 of the License, or (at your option) any later version.
+##
+## This library is distributed in the hope that it will be useful,
+## but WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+## Lesser General Public License for more details.
+##
+## You should have received a copy of the GNU Lesser General Public
+## License along with this library. If not, see
+## <http://www.gnu.org/licenses/>.
+
+INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include
+LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la \
+ $(COVERAGE_LDFLAGS)
+
+noinst_PROGRAMS=domtop
+
+domtop_SOURCES=domtop.c
+domtop_LDFLAGS=
+domtop_LDADD= $(LDADDS)
diff --git a/examples/domtop/domtop.c b/examples/domtop/domtop.c
new file mode 100644
index 000..fcdcc66
--- /dev/null
+++ b/examples/domtop/domtop.c
@@ -0,0 +1,388 @@
+/*
+ * domtop.c: Demo program showing how to calculate CPU usage
+ *
+ * Copyright (C) 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
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licenses/>.
+ *
+ * Author: Michal Privoznik
+ */
+
+#include
+#include
+#include
On 11.07.2014 19:47, Michele Paolino wrote:
> This patch adds support for the QEMU vhost-user feature to libvirt.
> vhost-user enables the communication between a QEMU virtual machine
> and other userspace process using the Virtio transport protocol.
> It uses a char dev (e.g. Unix socket) for the control plane,
> while the data plane based on shared memory.
>
> The XML looks like:
>
>
>
>
>
>
>
> changes from v1:
> * addressed comments
> * removed unnecessary checks
> * series merged in a single patch
We tend to write the diff to previous versions into notes not in the commit
message as it pollutes git log.
BTW: I didn't ask the whole patchset to be merged into a single patch, but it
doesn't hurt in this specific case either (the diff stat seems reasonably big).
>
> The previous version of this patch can be found at:
> http://www.redhat.com/archives/libvir-list/2014-July/msg00111.html
>
> Signed-off-by: Michele Paolino
> ---
> docs/formatdomain.html.in | 34 +
> docs/schemas/domaincommon.rng | 25 +++
> src/conf/domain_conf.c | 87
> ++
> src/conf/domain_conf.h | 10 ++-
> src/libxl/libxl_conf.c | 1 +
> src/lxc/lxc_process.c | 1 +
> src/qemu/qemu_command.c| 63
> src/uml/uml_conf.c | 5 ++
> src/xenxs/xen_sxpr.c | 1 +
> .../qemuxml2argv-net-vhostuser.args| 7 ++
> .../qemuxml2argv-net-vhostuser.xml | 33
> tests/qemuxml2argvtest.c | 1 +
> tests/qemuxml2xmltest.c| 1 +
> 13 files changed, 267 insertions(+), 2 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-net-vhostuser.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 3f8bbee..606b7d4 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -3927,6 +3927,40 @@ qemu-kvm -net nic,model=? /dev/null
> Since 0.9.5
>
>
> +vhost-user interface
> +
> +
> + vhost-user enables the communication between a QEMU virtual machine
> + and other userspace process using the Virtio transport protocol.
> + A char dev (e.g. Unix socket) is used for the control plane, while
> + the data plane is based on shared memory.
> +
> +
> +
> + ...
> +
> +
> +
> +
> +
> +
> +
> +
I don't think so. Empty bodies elements are written as . And that's how
libvirt formats them too. And if I were to be really picky, is formated
before .
> +
> +
> + ...
> +
> +
> + The element has to be specified
> + along with the type of char device.
> + Currently, only type='unix' is supported, where the path (the
> + directory path of the socket) and mode attributes are required.
> + Both mode='server' and mode='client'
> + are supported.
> + vhost-user requires the virtio model type, thus the
> + element is mandatory.
> +
I think it would be useful for users reading the documentation to know what
version of libvirt was this introduced in.
> +
> Input devices
>
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index af51eee..c9c02b6 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1968,6 +1968,31 @@
>
>
>
> +vhostuser
> +
> +
> +
> +
> +
> +unix
> +
> +
> +
> +
> +
> +
> +
> +server
> +client
> +
> +
> +
> +
> +
> +
> +
> +
> +
> network
>
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 8df43b7..fb286c6 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -349,6 +349,7 @@ VIR_ENUM_IMPL(virDomainFSWrpolicy,
> VIR_DOMAIN_FS_WRPOLICY_LAST,
> VIR_ENUM_IMPL(virDomainNet, VIR_DOMAIN_NET_TYPE_LAST,
> "user",
> "ethernet",
> + "vhostuser",
> "server",
> "client",
> "mcast",
> @@ -1361,6 +1362,1
On 16.07.2014 21:00, Eric Blake wrote:
We have some inconsistencies in the node capabilities (which shows guest
capabilities for some default binaries) and domcapabilities (which shows
guest capabilities for a specified binary). It might be nicer for
client applications if the two XML components share a common schema and
output layout, so that the same client code can be used to parse either
(sub-tree of) XML, modulo the name of the top-most element containing
the tree.
Furthermore, I'm trying to figure out how to advertise whether a given
domain will support active commit (and similarly, Peter's patches for
relative backing name preservation). Advertising the feature just
through 'virsh capabilities' is insufficient, because that only covers
the default binary; so it seems like the sort of thing that should also
be in 'virsh domcapabilities'.
That depends on how's active commit accepted by libvirt. IIUC it's a
flag to an API. (Okay, you got me there, I'm not paying much attention
to snapshot work).
The best solution would be to introduce another section, where supported
flags to APIs would be enumerated (same way that attribute values are).
But this is sooo much more work than in attribute part (esp. without
introspection) that the resulting code would be unmaintainable.
So my suggestion is to come up with yet another section and put
arbitrary strings there to represent features like active commit. For
instance:
...
active-commit
{some ordinary commit}
Since virConnectGetDomainCapabilities is brand new to 1.2.7, we still
have time to change its XML. But I want consensus on whether we need
things to match, or whether we intentionally want people to rely only on
the newer XML format of the new API call (that is, don't bloat 'virsh
capabilities'/virConnectGetCapabilities any further, and learning
whether active commit is supported will have to be done via 'virsh
domcapabilities'/virConnectGetDomainCapabilities). That is, I'm
wondering if should use / as
its starting point, rather than completely inventing new XML.
I'm also finding 'virsh domcapabilities' a bit hard to use - even though
it allows all its arguments to be optional at the RPC level, the qemu
implementation isn't so happy:
# tools/virsh domcapabilities
error: failed to get emulator capabilities
error: virttype_str in qemuConnectGetDomainCapabilities must not be NULL
but how am I supposed to know what --virttype strings are valid?
By reading the documentation :P
From the virsh manpage:
The virttype option specifies the virtualization type used. The value to
be used is either from the 'type' attribute of the top level
element from the domain XML or the 'type' attribute found within each
element from the virsh capabilities output.
I know virsh user is user unfriendly, but I think this could be solved
by wise auto completion (if I find another student to complete it). Or
if you have any idea meanwhile ...
# tools/virsh domcapabilities --virttype kvm
error: failed to get emulator capabilities
error: invalid argument: at least one of emulatorbin or architecture
fields must be present
Would it be nicer to behave the same as 'virsh capabilities' and give
the details of the default binary in this case?
Sure, but in order to get default binary we must know architecture
(consider the case where you have both /usr/bin/qemu-system-x86_64 and
/usr/bin/qemu-system-i686). Although, having only one qemu binary on the
system makes it easy to find the default, doesn't it? Patch on the way.
Now, for a comparison between the two XML per binary:
'virsh capabilities' gives me this segment:
hvm
64
/usr/bin/qemu-system-alpha
clipper
while 'virsh domcapabilities --emulatorbin /usr/bin/qemu-system-alpha
--virttype kvm' gives this:
/usr/bin/qemu-system-alpha
kvm
clipper
alpha
disk
cdrom
floppy
lun
ide
fdc
scsi
virtio
usb
subsystem
default
mandatory
requisite
optional
usb
pci
scsi
I'm okay that the domcapabilites output is longer, and don't think we
need to backport any of the new stuff to the older API. But any
information present in the old API should be easily retrieved using the
new API, so that the information isn't lost, and so that a client can
learn the same amount of detail about a non-default binary as they can
about the defaults.
Look at the difference in XPath to get to some of the same information:
/guest/os_type vs. ? - where is os_type in domcapabilities?
nowhere yet.
/guest/arch@name vs. /domainCapabilities/arch - why is one an attribute
and the other an element?
/
Up to now, users have to pass two arguments at least: domain virt type
('qemu' vs 'kvm') and one of emulatorbin or architecture. This is not
much user friendly. Nowadays users mostly use KVM and share the host
architecture with the guest. So now, the API (and subsequently virsh
command) can be called with all NULLs (without any arguments).
Before this patch:
# virsh domcapabilities
error: failed to get emulator capabilities
error: virttype_str in qemuConnectGetDomainCapabilities must not be NULL
# virsh domcapabilities kvm
error: failed to get emulator capabilities
error: invalid argument: at least one of emulatorbin or architecture fields
must be present
After:
# virsh domcapabilities
/usr/bin/qemu-system-x86_64
kvm
pc-i440fx-2.1
x86_64
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_driver.c | 24 ++--
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 33541d3..7d99435 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16849,17 +16849,17 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
char *ret = NULL;
virQEMUDriverPtr driver = conn->privateData;
virQEMUCapsPtr qemuCaps = NULL;
-int virttype; /* virDomainVirtType */
+int virttype = VIR_DOMAIN_VIRT_KVM; /* virDomainVirtType */
virDomainCapsPtr domCaps = NULL;
-int arch = VIR_ARCH_NONE; /* virArch */
+int arch = virArchFromHost(); /* virArch */
virCheckFlags(0, ret);
-virCheckNonNullArgReturn(virttype_str, ret);
if (virConnectGetDomainCapabilitiesEnsureACL(conn) < 0)
return ret;
-if ((virttype = virDomainVirtTypeFromString(virttype_str)) < 0) {
+if (virttype_str &&
+(virttype = virDomainVirtTypeFromString(virttype_str)) < 0) {
virReportError(VIR_ERR_INVALID_ARG,
_("unknown virttype: %s"),
virttype_str);
@@ -16882,9 +16882,6 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
arch_from_caps = virQEMUCapsGetArch(qemuCaps);
-if (arch == VIR_ARCH_NONE)
-arch = arch_from_caps;
-
if (arch_from_caps != arch) {
virReportError(VIR_ERR_INVALID_ARG,
_("architecture from emulator '%s' doesn't "
@@ -16893,21 +16890,12 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
virArchToString(arch));
goto cleanup;
}
-} else if (arch_str) {
+} else {
if (!(qemuCaps = virQEMUCapsCacheLookupByArch(driver->qemuCapsCache,
arch)))
goto cleanup;
-if (!emulatorbin)
-emulatorbin = virQEMUCapsGetBinary(qemuCaps);
-/* Deliberately not checking if provided @emulatorbin matches @arch,
- * since if @emulatorbin was specified the match has been checked a few
- * lines above. */
-} else {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("at least one of emulatorbin or "
- "architecture fields must be present"));
-goto cleanup;
+emulatorbin = virQEMUCapsGetBinary(qemuCaps);
}
if (machine) {
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 17.07.2014 11:11, Daniel P. Berrange wrote:
On Thu, Jul 17, 2014 at 11:05:08AM +0200, Michal Privoznik wrote:
# tools/virsh domcapabilities --virttype kvm
error: failed to get emulator capabilities
error: invalid argument: at least one of emulatorbin or architecture
fields must be present
Would it be nicer to behave the same as 'virsh capabilities' and give
the details of the default binary in this case?
Sure, but in order to get default binary we must know architecture (consider
the case where you have both /usr/bin/qemu-system-x86_64 and
/usr/bin/qemu-system-i686). Although, having only one qemu binary on the
system makes it easy to find the default, doesn't it? Patch on the way.
IMHO it would be preferrable to always default to virArchFromHost()
if arch is none, since that gives a predictable default value, as
opposed to probing emulators which is unpredictable
Yep, that's exactly what I've done in my patch:
https://www.redhat.com/archives/libvir-list/2014-July/msg00884.html
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Signed-off-by: Michal Privoznik
---
docs/formatdomain.html.in | 18 +-
docs/schemas/domaincommon.rng | 19 +-
src/conf/domain_conf.c | 197 +++--
src/conf/domain_conf.h | 13 +-
src/parallels/parallels_driver.c | 2 +-
src/qemu/qemu_command.c| 2 +-
src/qemu/qemu_conf.c | 20 ++-
src/qemu/qemu_process.c| 2 +-
.../qemuxml2argv-hugepages-pages.xml | 45 +
tests/qemuxml2xmltest.c| 1 +
10 files changed, 288 insertions(+), 31 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 3c85fc5..f4362e6 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -617,7 +617,9 @@
<domain>
...
<memoryBacking>
-<hugepages/>
+<hugepages>
+ <page size="1" unit="G" nodeset="0-3,5"/>
+ <page size="2" unit="M" nodeset="4"/>
<nosharepages/>
<locked/>
</memoryBacking>
@@ -632,7 +634,19 @@
hugepages
This tells the hypervisor that the guest should have its memory
-allocated using hugepages instead of the normal native page size.
+ allocated using hugepages instead of the normal native page size.
+ Since 1.2.5 it's possible to set hugepages
+ more specifically per numa node. The page element is
+ introduced. It has one compulsory attribute size which
+ specifies which hugepages should be used (especially useful on systems
+ supporting hugepages of different sizes). The default unit for the
+ size attribute is kilobytes (multiplier of 1024). If you
+ want to use different unit, use optional unit attribute.
+ For systems with NUMA, the optional nodeset attribute may
+ come handy as it ties given guest's NUMA nodes to certain hugepage
+ sizes. From the example snippet, one gigabyte hugepages are used for
+ every NUMA node except node number four. For the correct syntax see
+ this.
nosharepages
Instructs hypervisor to disable shared pages (memory merge, KSM) for
this domain. Since 1.0.6
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 2caeef9..d9da0bc 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -567,7 +567,24 @@
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a1ef374..b49bcb0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11258,6 +11258,57 @@ virDomainParseMemory(const char *xpath,
xmlXPathContextPtr ctxt,
}
+static int
+virDomainHugepagesParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virDomainHugePagePtr hugepage)
+{
+int ret = -1;
+xmlNodePtr oldnode = ctxt->node;
+unsigned long long bytes, max;
+char *unit = NULL, *nodeset = NULL;
+
+ctxt->node = node;
+
+/* On 32-bit machines, our bound is 0x * KiB. On 64-bit
+ * machines, our bound is off_t (2^63). */
+if (sizeof(unsigned long) < sizeof(long long))
+max = 1024ull * ULONG_MAX;
+else
+max = LLONG_MAX;
+
+if (virXPathULongLong("string(./@size)", ctxt, &bytes) < 0) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("unable to parse size attribute"));
+goto cleanup;
+}
+
+unit = virXPathString("string(./@unit)", ctxt);
+
+if (virScaleInteger(&bytes, unit, 1024, max) < 0)
+goto cleanup;
+
+if (!(hugepage->size = VIR_DIV_UP(bytes, 1024))) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("hugepage size can't be zero"));
+goto cleanup;
+}
+
+if ((nodeset = virXMLPropString(node, "nodeset"))) {
+if (virBitmapParse(nodeset, 0, &hugepage->nodemask,
+ VIR_DOMAIN_CPUMASK_LEN) < 0)
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(unit);
+VIR_FREE(nodeset);
+ctxt->node = o
There's this question on the list that is asked over and over again.
How do I get {cpu, memory, ...} usage in percentage? Or its modified
version: How do I plot nice graphs like virt-manager does?
It would be nice if we have an example to inspire people. And that's
what domtop should do. Yes, it could be written in different ways, but
I've chosen this one as I think it show explicitly what users need to
implement in order to imitate virt-manager's graphing.
Note: The usage is displayed from host perspective. That is, how much
host CPUs the domain is using. But it should be fairly simple to
switch do just guest CPU usage if needed.
Signed-off-by: Michal Privoznik
---
.gitignore | 1 +
Makefile.am | 2 +-
cfg.mk | 2 +-
configure.ac| 1 +
examples/domtop/Makefile.am | 27 +++
examples/domtop/domtop.c| 400
libvirt.spec.in | 2 +-
7 files changed, 432 insertions(+), 3 deletions(-)
create mode 100644 examples/domtop/Makefile.am
create mode 100644 examples/domtop/domtop.c
diff --git a/.gitignore b/.gitignore
index 2d4d401..90fee91 100644
--- a/.gitignore
+++ b/.gitignore
@@ -75,6 +75,7 @@
/examples/dominfo/info1
/examples/domsuspend/suspend
/examples/dommigrate/dommigrate
+/examples/domtop/domtop
/examples/hellolibvirt/hellolibvirt
/examples/openauth/openauth
/gnulib/lib/*
diff --git a/Makefile.am b/Makefile.am
index a374e1a..4aafe94 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -24,7 +24,7 @@ SUBDIRS = . gnulib/lib include src daemon tools docs
gnulib/tests \
examples/dominfo examples/domsuspend examples/apparmor \
examples/xml/nwfilter examples/openauth examples/systemtap \
tools/wireshark examples/dommigrate \
- examples/lxcconvert
+ examples/lxcconvert examples/domtop
ACLOCAL_AMFLAGS = -I m4
diff --git a/cfg.mk b/cfg.mk
index c3d89a0..0559edd 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -1078,7 +1078,7 @@ exclude_file_name_regexp--sc_prohibit_sprintf = \
exclude_file_name_regexp--sc_prohibit_strncpy = ^src/util/virstring\.c$$
exclude_file_name_regexp--sc_prohibit_strtol = \
- ^(src/util/.*|examples/domsuspend/suspend)\.c$$
+ ^(src/util/.*|examples/dom.*/.*)\.c$$
exclude_file_name_regexp--sc_prohibit_xmlGetProp = ^src/util/virxml\.c$$
diff --git a/configure.ac b/configure.ac
index 8001e24..f37c716 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2755,6 +2755,7 @@ AC_CONFIG_FILES([\
examples/domsuspend/Makefile \
examples/dominfo/Makefile \
examples/dommigrate/Makefile \
+examples/domtop/Makefile \
examples/openauth/Makefile \
examples/hellolibvirt/Makefile \
examples/systemtap/Makefile \
diff --git a/examples/domtop/Makefile.am b/examples/domtop/Makefile.am
new file mode 100644
index 000..c5cb6c7
--- /dev/null
+++ b/examples/domtop/Makefile.am
@@ -0,0 +1,27 @@
+## Process this file with automake to produce Makefile.in
+
+## Copyright (C) 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
+## License as published by the Free Software Foundation; either
+## version 2.1 of the License, or (at your option) any later version.
+##
+## This library is distributed in the hope that it will be useful,
+## but WITHOUT ANY WARRANTY; without even the implied warranty of
+## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+## Lesser General Public License for more details.
+##
+## You should have received a copy of the GNU Lesser General Public
+## License along with this library. If not, see
+## <http://www.gnu.org/licenses/>.
+
+INCLUDES = -I$(top_builddir)/include -I$(top_srcdir)/include
+LDADDS = $(STATIC_BINARIES) $(WARN_CFLAGS) $(top_builddir)/src/libvirt.la \
+ $(COVERAGE_LDFLAGS)
+
+noinst_PROGRAMS=domtop
+
+domtop_SOURCES=domtop.c
+domtop_LDFLAGS=
+domtop_LDADD= $(LDADDS)
diff --git a/examples/domtop/domtop.c b/examples/domtop/domtop.c
new file mode 100644
index 000..6fb6cbe
--- /dev/null
+++ b/examples/domtop/domtop.c
@@ -0,0 +1,400 @@
+/*
+ * domtop.c: Demo program showing how to calculate CPU usage
+ *
+ * Copyright (C) 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
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library. If not, see
+ * <http://www.gnu.org/licen
On 17.07.2014 11:28, Daniel P. Berrange wrote:
> On Thu, Jul 17, 2014 at 11:12:05AM +0200, Michal Privoznik wrote:
>> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>> index 33541d3..7d99435 100644
>> --- a/src/qemu/qemu_driver.c
>> +++ b/src/qemu/qemu_driver.c
>> @@ -16849,17 +16849,17 @@ qemuConnectGetDomainCapabilities(virConnectPtr
>> conn,
>> char *ret = NULL;
>> virQEMUDriverPtr driver = conn->privateData;
>> virQEMUCapsPtr qemuCaps = NULL;
>> -int virttype; /* virDomainVirtType */
>> +int virttype = VIR_DOMAIN_VIRT_KVM; /* virDomainVirtType */
>
> Isn't this going to always fail when we run inside a guest which does not
> have nested virt? When we know the QEMU binary, we probe to see whether
> it can do KVM or not, so could we postpone this decision about virt type
> until after we've checked if KVM was available ?
I don't think we can. I mean, in order to construct virDomainCaps object we
need to know the virttype. The KVM is checked later in the process when the
object exits already. What we can do, however, is check for KVM twice. How
about this squashed in?
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 7d99435..33d6e47 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16849,7 +16849,7 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
char *ret = NULL;
virQEMUDriverPtr driver = conn->privateData;
virQEMUCapsPtr qemuCaps = NULL;
-int virttype = VIR_DOMAIN_VIRT_KVM; /* virDomainVirtType */
+int virttype; /* virDomainVirtType */
virDomainCapsPtr domCaps = NULL;
int arch = virArchFromHost(); /* virArch */
@@ -16858,6 +16858,11 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
if (virConnectGetDomainCapabilitiesEnsureACL(conn) < 0)
return ret;
+if (qemuHostdevHostSupportsPassthroughLegacy())
+virttype = VIR_DOMAIN_VIRT_KVM;
+else
+virttype = VIR_DOMAIN_VIRT_QEMU;
+
if (virttype_str &&
(virttype = virDomainVirtTypeFromString(virttype_str)) < 0) {
virReportError(VIR_ERR_INVALID_ARG,
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 14.07.2014 16:12, Paolo Bonzini wrote:
Il 14/07/2014 11:27, Daniel P. Berrange ha scritto:
-drive file=img_1,if=pflash,format=raw,readonly \
-drive file=img_2,if=pflash,format=raw
It's safer to add ",unit=0" and ",unit=1" too.
We already use for specifying alternative BIOS blobs for
the QEMU -bios arg. Since you say this obsoletes the -bios arg, I
think it makes sense to use for the read-only firmware
image.
It obsoletes the -bios argument, but it is not the same thing:
1) on some machines you can use "-drive if=pflash" for the nvram, but
not for the firmware
And how should the FW cmd line look like then?
2) on some older versions of QEMU, "-drive if=pflash" will work only on
TCG or will not work at all so you cannot blindly replace it.
Whoa. How to detect this? I mean, how do detect both?
What about:
...
...
where the mapping from to -drive if=flash is partly
machine-dependent. On x86, for example, adds the ",unit=1"
sub-option and fails if the element is absent; it also fails if
has type='rom'.
Makes sense.
For the variable storage, I'd probably suggest as the
element name, since IIUC that's a fairly commonly used term for
this concept.
I like this.
Additionally it would be great if we'd be able to
generate an empty nvram for a guest if the user doesn't specify it.
I'm not big fan of this. Pre-creating storage is something that should
be done by mgmt applications (migration with non-shared storage is
something different).
Laszlo has OVMF patches to "auto-format" an all-zero nvram file.
Great. Do they work automagically or does libvirt need to enable the
formatting somehow (e.g. monitor command, cmd line argument, ...)?
Paolo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08.07.2014 13:54, John Ferlan wrote:
Reposting of a series from last month changing only the span version
in the docs from 1.2.6 to 1.2.7. Previous posting here:
http://www.redhat.com/archives/libvir-list/2014-June/msg00448.html
The concept still remains the same - rather than rely on the hostNN
numbers for the scsi_host to remain stable and unique across host reboots
and/or kernel rebuilds, allow use a combination of the scsi_host's PCI
address and the value from the hostNN's 'unique_id' file.
John Ferlan (6):
getAdapterName: check for SCSI_HOST
scsi_backend: Use existing LINUX_SYSFS_SCSI_HOST_PREFIX definition
virutil: Introduce virReadSCSIUniqueId
Add unique_id to nodedev output
scsi_host: Introduce virFindSCSIHostByPCI
getAdapterName: Lookup stable scsi_host
Osier Yang (2):
virStoragePoolSourceAdapter: Refine the SCSI_HOST adapter name
storage: Introduce parentaddr into virStoragePoolSourceAdapter
docs/formatnode.html.in| 11 +
docs/formatstorage.html.in | 143 --
docs/schemas/basictypes.rng| 24 +-
docs/schemas/nodedev.rng | 6 +
src/conf/node_device_conf.c| 23 +-
src/conf/node_device_conf.h| 1 +
src/conf/storage_conf.c| 111 +++-
src/conf/storage_conf.h| 8 +-
src/libvirt_private.syms | 2 +
src/node_device/node_device_linux_sysfs.c | 6 +
src/phyp/phyp_driver.c | 8 +-
src/storage/storage_backend_scsi.c | 53 +++-
src/test/test_driver.c | 5 +-
src/util/virutil.c | 154 +++
src/util/virutil.h | 8 +
tests/Makefile.am | 7 +
.../pci_8086_27c5_scsi_host_0_unique_id.xml| 8 +
tests/nodedevxml2xmltest.c | 1 +
tests/scsihosttest.c | 308 +
.../pool-scsi-type-scsi-host-stable.xml| 19 ++
.../pool-scsi-type-scsi-host-stable.xml| 22 ++
tests/storagepoolxml2xmltest.c | 1 +
22 files changed, 868 insertions(+), 61 deletions(-)
create mode 100644
tests/nodedevschemadata/pci_8086_27c5_scsi_host_0_unique_id.xml
create mode 100644 tests/scsihosttest.c
create mode 100644
tests/storagepoolxml2xmlin/pool-scsi-type-scsi-host-stable.xml
create mode 100644
tests/storagepoolxml2xmlout/pool-scsi-type-scsi-host-stable.xml
ACK series, but please see my inline comments.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 08.07.2014 13:54, John Ferlan wrote:
Rather than assume that NOT FC_HOST is SCSI_HOST, let's call them out
specifically. Makes it easier to find SCSI_HOST code/structs and ensures
something isn't missed in the future
Signed-off-by: John Ferlan
---
src/storage/storage_backend_scsi.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/src/storage/storage_backend_scsi.c
b/src/storage/storage_backend_scsi.c
index f6f3ca2..21c13e5 100644
--- a/src/storage/storage_backend_scsi.c
+++ b/src/storage/storage_backend_scsi.c
@@ -547,18 +547,17 @@ getAdapterName(virStoragePoolSourceAdapter adapter)
{
char *name = NULL;
-if (adapter.type != VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_SCSI_HOST) {
ignore_value(VIR_STRDUP(name, adapter.data.name));
-return name;
-}
-
-if (!(name = virGetFCHostNameByWWN(NULL,
- adapter.data.fchost.wwnn,
- adapter.data.fchost.wwpn))) {
-virReportError(VIR_ERR_XML_ERROR,
- _("Failed to find SCSI host with wwnn='%s', "
- "wwpn='%s'"), adapter.data.fchost.wwnn,
- adapter.data.fchost.wwpn);
+} else if (adapter.type == VIR_STORAGE_POOL_SOURCE_ADAPTER_TYPE_FC_HOST) {
+if (!(name = virGetFCHostNameByWWN(NULL,
+ adapter.data.fchost.wwnn,
+ adapter.data.fchost.wwpn))) {
+virReportError(VIR_ERR_XML_ERROR,
+ _("Failed to find SCSI host with wwnn='%s', "
+ "wwpn='%s'"), adapter.data.fchost.wwnn,
+ adapter.data.fchost.wwpn);
+}
}
return name;
Or even better use switch((virStoragePoolSourceAdapterType)
adapter.type) {}. But I can live with this version too.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 18.07.2014 15:48, Eric Blake wrote:
On 07/18/2014 03:08 AM, Michal Privoznik wrote:
There's this question on the list that is asked over and over again.
How do I get {cpu, memory, ...} usage in percentage? Or its modified
version: How do I plot nice graphs like virt-manager does?
It would be nice if we have an example to inspire people. And that's
what domtop should do. Yes, it could be written in different ways, but
I've chosen this one as I think it show explicitly what users need to
implement in order to imitate virt-manager's graphing.
Note: The usage is displayed from host perspective. That is, how much
host CPUs the domain is using. But it should be fairly simple to
switch do just guest CPU usage if needed.
Signed-off-by: Michal Privoznik
---
ACK with nits fixed.
Fixed and pushed. Thanks!
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
In the fbd91d49 commit, new scsihostdata dir is added to EXTRA_DIST in
the tests/Makefile.am. However, the directory itself is not created
anywhere, nor in the commit.
Signed-off-by: Michal Privoznik
---
Notes:
Pushed under build-breaker rule.
tests/Makefile.am | 1 -
1 file changed, 1 deletion(-)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index ecb2f34..3e71069 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -83,7 +83,6 @@ EXTRA_DIST = \
domainsnapshotxml2xmlin \
domainsnapshotxml2xmlout \
fchostdata \
- scsihostdata \
interfaceschemadata \
lxcconf2xmldata \
lxcxml2xmldata \
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 21.07.2014 17:09, Daniel P. Berrange wrote:
On Thu, Jul 17, 2014 at 06:12:46PM +0200, Michal Privoznik wrote:
+
+ QEMUGuest1
+ c7a5fdbd-edaf-9455-926a-d65c16db1809
+ 4194304
+ 4194304
+
+
+
+
+
+
+ 4
+
+
+
+
+
+hvm
+
+
+
+
+
+
+
+
+
+
There's nothing functionally wrong with what you have here, but I'm
wondering if you considered just adding a page size attribute against
the element under here ? Feels like that might be a bit
less verbose for the XML
Huh funny. That idea came up to my mind, but I thought it was more
verbose so I went down this road. I'm not fundamentally against it, but
I like my approach more. In most common case, users will use only one
size of huge pages to back their guests, so all they need to do is:
instead of repeating @pagesize attribute in each . But as far as
I see it's just question of preference without any technical impact, right?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Use better detection of hugetlbfs mount points. Yes, there can be
multiple mount points each serving different huge page size.
Since we already have ability to override the mount point in the
qemu.conf file, this crazy backward compatibility code is brought in.
Now we allow multiple mount points, so the "hugetlbfs_mount" option
must take an list of strings (mount points). But previously, it was
just a string, so we must accept both types now.
Signed-off-by: Michal Privoznik
---
Notes:
Yes, the libvirtd_qemu.aug change is missing here. The reason is prosaic: I
don't know how to make augeas accept both array of string and a single
string
for a configuration variable. BTW: the security_drivers which does exactly
what
I'm doing here, accepts only single string in the aug file too.
src/Makefile.am | 12 ++---
src/qemu/qemu.conf | 9 +++-
src/qemu/qemu_command.c | 20 +---
src/qemu/qemu_conf.c | 124 ++-
src/qemu/qemu_conf.h | 9 +++-
src/qemu/qemu_driver.c | 39 +++
src/qemu/qemu_process.c | 21 +---
tests/qemuxml2argvtest.c | 10 ++--
8 files changed, 183 insertions(+), 61 deletions(-)
diff --git a/src/Makefile.am b/src/Makefile.am
index 982f63d..2444d51 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -1209,10 +1209,10 @@ libvirt_driver_qemu_impl_la_CFLAGS = \
$(AM_CFLAGS)
libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS)
libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \
-$(GNUTLS_LIBS) \
- $(LIBNL_LIBS) \
- $(LIBXML_LIBS) \
- $(NULL)
+$(GNUTLS_LIBS) \
+$(LIBNL_LIBS) \
+$(LIBXML_LIBS) \
+libvirt_util.la
libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES)
conf_DATA += qemu/qemu.conf
@@ -1995,8 +1995,8 @@ endif WITH_DRIVER_MODULES
BUILT_SOURCES += libvirt_probes.h libvirt_probes.stp libvirt_functions.stp
if WITH_QEMU
-libvirt_driver_qemu_la_LIBADD += libvirt_qemu_probes.lo
-nodist_libvirt_driver_qemu_la_SOURCES = libvirt_qemu_probes.h
+libvirt_driver_qemu_impl_la_LIBADD += libvirt_qemu_probes.lo libvirt_probes.lo
+libvirt_driver_qemu_impl_la_SOURCES += libvirt_qemu_probes.h
BUILT_SOURCES += libvirt_qemu_probes.h
endif WITH_QEMU
diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf
index 18ce2a8..a079b93 100644
--- a/src/qemu/qemu.conf
+++ b/src/qemu/qemu.conf
@@ -330,7 +330,14 @@
# unspecified here, determination of a host mount point in /proc/mounts
# will be attempted. Specifying an explicit mount overrides detection
# of the same in /proc/mounts. Setting the mount point to "" will
-# disable guest hugepage backing.
+# disable guest hugepage backing. If desired, multiple mount points can
+# be specified at once, separated by comma and enclosed in square
+# brackets, for example:
+#
+# hugetlbfs_mount = ["/dev/hugepages2M", "/dev/hugepages1G"]
+#
+# The size of huge page served by specific mount point is determined by
+# libvirt at the daemon startup.
#
# NB, within this mount point, guests will create memory backing files
# in a location of $MOUNTPOINT/libvirt/qemu
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index fe207a4..091447a 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -7333,14 +7333,12 @@ qemuBuildCommandLine(virConnectPtr conn,
def->mem.max_balloon = VIR_DIV_UP(def->mem.max_balloon, 1024) * 1024;
virCommandAddArgFormat(cmd, "%llu", def->mem.max_balloon / 1024);
if (def->mem.hugepage_backed) {
-if (!cfg->hugetlbfsMount) {
+char *mem_path;
+
+if (!cfg->nhugetlbfs) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("hugetlbfs filesystem is not mounted"));
-goto error;
-}
-if (!cfg->hugepagePath) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- "%s", _("hugepages are disabled by administrator
config"));
+ "%s", _("hugetlbfs filesystem is not mounted "
+ "or disabled by administrator config"));
goto error;
}
if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MEM_PATH)) {
@@ -7349,8 +7347,14 @@ qemuBuildCommandLine(virConnectPtr conn,
def->emulator);
goto error;
}
+
+if (!(mem_path = qemuGetDefaultHugepath(cfg->hugetlbfs,
+cfg->nhugetlbfs)))
+goto error;
+
virCommandAddArgList(cmd, "-mem-prealloc", "-mem-path",
- cfg->hugepagePath, NULL);
+ mem_path
Signed-off-by: Michal Privoznik
---
docs/formatdomain.html.in | 18 +-
docs/schemas/domaincommon.rng | 19 +-
src/conf/domain_conf.c | 197 +++--
src/conf/domain_conf.h | 13 +-
src/parallels/parallels_driver.c | 2 +-
src/qemu/qemu_command.c| 2 +-
src/qemu/qemu_process.c| 2 +-
.../qemuxml2argv-hugepages-pages.xml | 45 +
tests/qemuxml2xmltest.c| 1 +
9 files changed, 277 insertions(+), 22 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-hugepages-pages.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 8950959..bce5885 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -617,7 +617,9 @@
<domain>
...
<memoryBacking>
-<hugepages/>
+<hugepages>
+ <page size="1" unit="G" nodeset="0-3,5"/>
+ <page size="2" unit="M" nodeset="4"/>
<nosharepages/>
<locked/>
</memoryBacking>
@@ -632,7 +634,19 @@
hugepages
This tells the hypervisor that the guest should have its memory
-allocated using hugepages instead of the normal native page size.
+ allocated using hugepages instead of the normal native page size.
+ Since 1.2.5 it's possible to set hugepages
+ more specifically per numa node. The page element is
+ introduced. It has one compulsory attribute size which
+ specifies which hugepages should be used (especially useful on systems
+ supporting hugepages of different sizes). The default unit for the
+ size attribute is kilobytes (multiplier of 1024). If you
+ want to use different unit, use optional unit attribute.
+ For systems with NUMA, the optional nodeset attribute may
+ come handy as it ties given guest's NUMA nodes to certain hugepage
+ sizes. From the example snippet, one gigabyte hugepages are used for
+ every NUMA node except node number four. For the correct syntax see
+ this.
nosharepages
Instructs hypervisor to disable shared pages (memory merge, KSM) for
this domain. Since 1.0.6
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index f6f697c..cf4cda8 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -567,7 +567,24 @@
-
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index d8d1fe7..ff50252 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11260,6 +11260,57 @@ virDomainParseMemory(const char *xpath,
xmlXPathContextPtr ctxt,
}
+static int
+virDomainHugepagesParseXML(xmlNodePtr node,
+ xmlXPathContextPtr ctxt,
+ virDomainHugePagePtr hugepage)
+{
+int ret = -1;
+xmlNodePtr oldnode = ctxt->node;
+unsigned long long bytes, max;
+char *unit = NULL, *nodeset = NULL;
+
+ctxt->node = node;
+
+/* On 32-bit machines, our bound is 0x * KiB. On 64-bit
+ * machines, our bound is off_t (2^63). */
+if (sizeof(unsigned long) < sizeof(long long))
+max = 1024ull * ULONG_MAX;
+else
+max = LLONG_MAX;
+
+if (virXPathULongLong("string(./@size)", ctxt, &bytes) < 0) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("unable to parse size attribute"));
+goto cleanup;
+}
+
+unit = virXPathString("string(./@unit)", ctxt);
+
+if (virScaleInteger(&bytes, unit, 1024, max) < 0)
+goto cleanup;
+
+if (!(hugepage->size = VIR_DIV_UP(bytes, 1024))) {
+virReportError(VIR_ERR_XML_DETAIL, "%s",
+ _("hugepage size can't be zero"));
+goto cleanup;
+}
+
+if ((nodeset = virXMLPropString(node, "nodeset"))) {
+if (virBitmapParse(nodeset, 0, &hugepage->nodemask,
+ VIR_DOMAIN_CPUMASK_LEN) < 0)
+goto cleanup;
+}
+
+ret = 0;
+ cleanup:
+VIR_FREE(unit);
+VIR_FREE(nodeset);
+ctxt->node = oldnode;
+return ret;
+}
+
+
static virDomainResourceDefPtr
Up to now, users have to pass two arguments at least: domain virt type
('qemu' vs 'kvm') and one of emulatorbin or architecture. This is not
much user friendly. Nowadays users mostly use KVM and share the host
architecture with the guest. So now, the API (and subsequently virsh
command) can be called with all NULLs (without any arguments).
Before this patch:
# virsh domcapabilities
error: failed to get emulator capabilities
error: virttype_str in qemuConnectGetDomainCapabilities must not be NULL
# virsh domcapabilities kvm
error: failed to get emulator capabilities
error: invalid argument: at least one of emulatorbin or architecture fields
must be present
After:
# virsh domcapabilities
/usr/bin/qemu-system-x86_64
kvm
pc-i440fx-2.1
x86_64
Signed-off-by: Michal Privoznik
---
Notes:
It would be nice to have this in the same release as the new API it's
fixing.
So please, review.
src/qemu/qemu_driver.c | 27 ++-
1 file changed, 10 insertions(+), 17 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b85d909..008f101 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16893,15 +16893,20 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
virQEMUCapsPtr qemuCaps = NULL;
int virttype; /* virDomainVirtType */
virDomainCapsPtr domCaps = NULL;
-int arch = VIR_ARCH_NONE; /* virArch */
+int arch = virArchFromHost(); /* virArch */
virCheckFlags(0, ret);
-virCheckNonNullArgReturn(virttype_str, ret);
if (virConnectGetDomainCapabilitiesEnsureACL(conn) < 0)
return ret;
-if ((virttype = virDomainVirtTypeFromString(virttype_str)) < 0) {
+if (qemuHostdevHostSupportsPassthroughLegacy())
+virttype = VIR_DOMAIN_VIRT_KVM;
+else
+virttype = VIR_DOMAIN_VIRT_QEMU;
+
+if (virttype_str &&
+(virttype = virDomainVirtTypeFromString(virttype_str)) < 0) {
virReportError(VIR_ERR_INVALID_ARG,
_("unknown virttype: %s"),
virttype_str);
@@ -16924,9 +16929,6 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
arch_from_caps = virQEMUCapsGetArch(qemuCaps);
-if (arch == VIR_ARCH_NONE)
-arch = arch_from_caps;
-
if (arch_from_caps != arch) {
virReportError(VIR_ERR_INVALID_ARG,
_("architecture from emulator '%s' doesn't "
@@ -16935,21 +16937,12 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
virArchToString(arch));
goto cleanup;
}
-} else if (arch_str) {
+} else {
if (!(qemuCaps = virQEMUCapsCacheLookupByArch(driver->qemuCapsCache,
arch)))
goto cleanup;
-if (!emulatorbin)
-emulatorbin = virQEMUCapsGetBinary(qemuCaps);
-/* Deliberately not checking if provided @emulatorbin matches @arch,
- * since if @emulatorbin was specified the match has been checked a few
- * lines above. */
-} else {
-virReportError(VIR_ERR_INVALID_ARG, "%s",
- _("at least one of emulatorbin or "
- "architecture fields must be present"));
-goto cleanup;
+emulatorbin = virQEMUCapsGetBinary(qemuCaps);
}
if (machine) {
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 16.07.2014 11:51, Chen Hanxiao wrote:
In lxc, we could not use setmaxmem command
with --config options.
This patch will add support for this.
Signed-off-by: Chen Hanxiao
---
src/lxc/lxc_driver.c | 69 ++--
1 file changed, 46 insertions(+), 23 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index b7b4b02..be6ee19 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -721,10 +721,10 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
virLXCDomainObjPrivatePtr priv;
virLXCDriverPtr driver = dom->conn->privateData;
virLXCDriverConfigPtr cfg = NULL;
-unsigned long oldmax = 0;
virCheckFlags(VIR_DOMAIN_AFFECT_LIVE |
- VIR_DOMAIN_AFFECT_CONFIG, -1);
+ VIR_DOMAIN_AFFECT_CONFIG |
+ VIR_DOMAIN_MEM_MAXIMUM, -1);
if (!(vm = lxcDomObjFromDomain(dom)))
goto cleanup;
@@ -743,32 +743,55 @@ static int lxcDomainSetMemoryFlags(virDomainPtr dom,
unsigned long newmem,
&persistentDef) < 0)
goto cleanup;
-if (flags & VIR_DOMAIN_AFFECT_LIVE)
-oldmax = vm->def->mem.max_balloon;
-if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
-if (!oldmax || oldmax > persistentDef->mem.max_balloon)
-oldmax = persistentDef->mem.max_balloon;
-}
+if (flags & VIR_DOMAIN_MEM_MAXIMUM) {
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+if (newmem < vm->def->mem.cur_balloon) {
+virReportError(VIR_ERR_OPERATION_INVALID, "%s",
+ _("Cannot resize the max memory less than current"
+ " memory for an active domain"));
+goto cleanup;
+}
+vm->def->mem.max_balloon = newmem;
Are things that easy? Don't we need to communicate this with the
lxc_controler somehow? Even though you allow only extending, I think
unless we are 100% sure guest will see the resize, we shouldn't allow this.
+}
-if (newmem > oldmax) {
-virReportError(VIR_ERR_INVALID_ARG,
- "%s", _("Cannot set memory higher than max memory"));
-goto cleanup;
-}
+if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+sa_assert(persistentDef);
Is this assert needed? Did clang complain or is this just a pure lefover
from copying from qemu_driver.c?
+persistentDef->mem.max_balloon = newmem;
+if (persistentDef->mem.cur_balloon > newmem)
+persistentDef->mem.cur_balloon = newmem;
+if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
+goto cleanup;
+}
+} else {
+unsigned long oldmax = 0;
-if (flags & VIR_DOMAIN_AFFECT_LIVE) {
-if (virCgroupSetMemory(priv->cgroup, newmem) < 0) {
-virReportError(VIR_ERR_OPERATION_FAILED,
- "%s", _("Failed to set memory for domain"));
-goto cleanup;
+if (flags & VIR_DOMAIN_AFFECT_LIVE)
+oldmax = vm->def->mem.max_balloon;
+if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+if (!oldmax || oldmax > persistentDef->mem.max_balloon)
+oldmax = persistentDef->mem.max_balloon;
}
-}
-if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
-sa_assert(persistentDef);
Well, since it has been here already, I think we can leave it in your
patch too.
-persistentDef->mem.cur_balloon = newmem;
-if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
+if (newmem > oldmax) {
+virReportError(VIR_ERR_INVALID_ARG,
+ "%s", _("Cannot set memory higher than max
memory"));
goto cleanup;
+}
+
+if (flags & VIR_DOMAIN_AFFECT_LIVE) {
+if (virCgroupSetMemory(priv->cgroup, newmem) < 0) {
+virReportError(VIR_ERR_OPERATION_FAILED,
+ "%s", _("Failed to set memory for domain"));
+goto cleanup;
+}
+}
+
+if (flags & VIR_DOMAIN_AFFECT_CONFIG) {
+sa_assert(persistentDef);
+persistentDef->mem.cur_balloon = newmem;
+if (virDomainSaveConfig(cfg->configDir, persistentDef) < 0)
+goto cleanup;
+}
}
ret = 0;
But what I miss in this patch is, what would:
virDomainSetMaxMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_CURRENT |
VIR_DOMAIN_MEM_MAXIMUM)
do? I mean, the _CURRENT is not translated to _LIVE or _CONFIG anywhere
in the function.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 16.07.2014 11:51, Chen Hanxiao wrote:
Signed-off-by: Chen Hanxiao
---
src/lxc/lxc_driver.c | 36 +---
1 file changed, 5 insertions(+), 31 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index be6ee19..9f974eb 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -680,37 +680,6 @@ lxcDomainGetMaxMemory(virDomainPtr dom)
return ret;
}
-static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long newmax)
-{
-virDomainObjPtr vm;
-int ret = -1;
-
-if (!(vm = lxcDomObjFromDomain(dom)))
-goto cleanup;
-
-if (virDomainSetMaxMemoryEnsureACL(dom->conn, vm->def) < 0)
-goto cleanup;
-
-if (newmax < vm->def->mem.cur_balloon) {
-if (!virDomainObjIsActive(vm)) {
-vm->def->mem.cur_balloon = newmax;
-} else {
-virReportError(VIR_ERR_OPERATION_INVALID, "%s",
- _("Cannot set max memory lower than current"
- " memory for an active domain"));
-goto cleanup;
-}
-}
-
-vm->def->mem.max_balloon = newmax;
-ret = 0;
-
- cleanup:
-if (vm)
-virObjectUnlock(vm);
-return ret;
-}
-
A-ha! This is what I was looking for in 1/2. Okay, but I'd rather note
this fact in 1/2 commit message to make it more obvious.
static int lxcDomainSetMemoryFlags(virDomainPtr dom, unsigned long newmem,
unsigned int flags)
{
@@ -809,6 +778,11 @@ static int lxcDomainSetMemory(virDomainPtr dom, unsigned
long newmem)
return lxcDomainSetMemoryFlags(dom, newmem, VIR_DOMAIN_AFFECT_LIVE);
}
+static int lxcDomainSetMaxMemory(virDomainPtr dom, unsigned long memory)
+{
+return lxcDomainSetMemoryFlags(dom, memory, VIR_DOMAIN_MEM_MAXIMUM);
+}
+
So previously, calling virDomainSetMaxMemory() on an inactive LXC domain
would succeed. Now, after the change, due to problem with _CURRENT,
_LIVE and _CONFIG this will basically return success, but without any
effect on the domain config. And that's wrong.
static int
lxcDomainSetMemoryParameters(virDomainPtr dom,
virTypedParameterPtr params,
Moreover, I think these two patches can be joined into one.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 07.07.2014 15:47, Cédric Bosdonnat wrote:
Here are a 2 fixes that make virt-sandbox-service work for me.
One allows it to work if selinux isn't handled by libvirtd, the other
safely handles some file copying that can be different across distros.
Cédric Bosdonnat (2):
virt-sandbox-service: check for security label only if they can be
handled
virt-sandbox-service: fix some paths for SUSE
bin/virt-sandbox-service | 42 --
1 file changed, 36 insertions(+), 6 deletions(-)
ACK to both patches.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 25.06.2014 19:22, Ján Tomko wrote:
Add a test to demonstrate the effect of the next patch.
---
.../qemuxml2argv-channel-virtio-autoassign.args| 20 +
.../qemuxml2argv-channel-virtio-autoassign.xml | 50 ++
tests/qemuxml2argvtest.c | 2 +
3 files changed, 72 insertions(+)
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.args
create mode 100644
tests/qemuxml2argvdata/qemuxml2argv-channel-virtio-autoassign.xml
ACK.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 25.06.2014 19:22, Ján Tomko wrote:
Over-engineered to allow non-contiguous indexes.
Free ports of a controller are stored in a virBitmap.
These bitmaps are stored in a hash table, indexed by
the controller index formatted as a string.
Buses are ignored for now, QEMU doesn't seem
to support anything else than .0
For virtconsoles, addresses are reserved silently,
since we don't put those in the XML, but they occupy
virtio-serial ports.
Missing:
auto-adding controllers when all ports are occupied
Fixes https://bugzilla.redhat.com/show_bug.cgi?id=890606
Paritally fixes https://bugzilla.redhat.com/show_bug.cgi?id=1076708
---
RFC, since it hashes the string of an integer.
Yeah, out hash tables require pointers, and we don't like to see an
integer taken as a pointer...
However, I like the idea (I mean the whole idea).
src/conf/domain_addr.c | 426 +
src/conf/domain_addr.h | 49 +++
src/conf/domain_conf.c | 30 --
src/libvirt_private.syms | 9 +
src/qemu/qemu_command.c| 61 +++
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_domain.h | 1 +
src/qemu/qemu_hotplug.c| 31 +-
tests/qemuhotplugtest.c| 2 +-
.../qemuxml2argv-channel-virtio-auto.args | 8 +-
.../qemuxml2argv-channel-virtio-autoassign.args| 10 +-
.../qemuxml2xmlout-channel-virtio-auto.xml | 10 +-
12 files changed, 591 insertions(+), 47 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 9cd6a3e..6e4c456 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -367,6 +367,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr
driver,
char *devstr = NULL;
qemuDomainObjPrivatePtr priv = vm->privateData;
bool releaseaddr = false;
+bool needs_remove = false;;
s/;;/;/
if (virDomainControllerFind(vm->def, controller->type, controller->idx)
>= 0) {
virReportError(VIR_ERR_OPERATION_FAILED,
I'd say ACK but since this is an RFC I let others to chime in.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
If user hasn't provided any @emulatorbin, the qemuCaps are
searched by @arch provided (which in fact can be guessed from the
host). However, there's no guarantee that the qemu binary for
@arch will exist. Therefore qemu capabilities may be nonexistent
too. If that's the case, we should throw an error message prior
jumping onto 'cleanup' label as the helper lookup function
remains silent on no search result.
Signed-off-by: Michal Privoznik
---
Notes:
This would qualify to be pushed under trivial rule, but maybe
somebody has a better idea for the error message (or its code).
src/qemu/qemu_driver.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 06d3f53..8e01965 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16990,8 +16990,12 @@ qemuConnectGetDomainCapabilities(virConnectPtr conn,
}
} else {
if (!(qemuCaps = virQEMUCapsCacheLookupByArch(driver->qemuCapsCache,
- arch)))
+ arch))) {
+virReportError(VIR_ERR_INVALID_ARG,
+ _("unable to find any emulator to serve '%s' "
+ "architecture"), virArchToString(arch));
goto cleanup;
+}
emulatorbin = virQEMUCapsGetBinary(qemuCaps);
}
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Currently, the function follows the usual pattern used in our code:
int ret = -1;
...
ret = 0;
cleanup:
return ret;
However, the function always call exit() on error, so the cleanup
label is never jumped onto. Therefore, it doesn't make any sense to
have the parse_argv function return an integer value, if it
effectively can return only value of zero.
Signed-off-by: Michal Privoznik
---
examples/domtop/domtop.c | 10 ++
1 file changed, 2 insertions(+), 8 deletions(-)
diff --git a/examples/domtop/domtop.c b/examples/domtop/domtop.c
index af5da46..204fdc3 100644
--- a/examples/domtop/domtop.c
+++ b/examples/domtop/domtop.c
@@ -94,13 +94,12 @@ print_usage(const char *progname)
unified_progname);
}
-static int
+static void
parse_argv(int argc, char *argv[],
const char **uri,
const char **dom_name,
unsigned int *milliseconds)
{
-int ret = -1;
int arg;
unsigned long val;
char *p;
@@ -155,10 +154,6 @@ parse_argv(int argc, char *argv[],
if (argc > optind)
*dom_name = argv[optind];
-
-ret = 0;
- cleanup:
-return ret;
}
static int
@@ -368,8 +363,7 @@ main(int argc, char *argv[])
unsigned int milliseconds = 500; /* Sleep this long between two API calls
*/
const int connect_flags = 0; /* No connect flags for now */
-if (parse_argv(argc, argv, &uri, &dom_name, &milliseconds) < 0)
-goto cleanup;
+parse_argv(argc, argv, &uri, &dom_name, &milliseconds);
DEBUG("Proceeding with uri=%s dom_name=%s milliseconds=%u",
uri, dom_name, milliseconds);
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
The variable 'k' in the print_cpu_usage function is not used anywhere
and can fire a warning on some compilers.
Signed-off-by: Michal Privoznik
---
examples/domtop/domtop.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/examples/domtop/domtop.c b/examples/domtop/domtop.c
index 204fdc3..e50988e 100644
--- a/examples/domtop/domtop.c
+++ b/examples/domtop/domtop.c
@@ -199,7 +199,7 @@ print_cpu_usage(const char *dom_name,
virTypedParameterPtr now_params,
size_t now_nparams)
{
-size_t i, j, k;
+size_t i, j;
size_t nparams = now_nparams;
bool delim = false;
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[Dropping libvirt-announce list]
On 05.08.2014 11:47, Himanshu Sharma wrote:
Dear Team,
Greetings!!
I'm testing libvirt with VMware ESXi. I'm able to connect to VMware ESXi with libvirt
driver but not able to connect it through "Virtual Machine manager" GUI also
not able to run virt-clone command inturn to clone ESXi VM. Can you please guide me how
to do so?
The virt-manager does not support ESX yet. It merely supports QEMU/KVM,
XEN and LXC. And for the virt-clone issue:
1) "computer broken" is usually not much descriptive. Is there any error
message you are seeing? What behavior are you experiencing?
2) virt tools have a separate mailing list:
http://www.redhat.com/mailman/listinfo/virt-tools-list
Try asking there.
Usually it's considered bad practice to repeat your question every 5
minutes, crosspost it over several mailing lists (even libvirt-announce
which is not user support channel but a low traffic list for
announcements as it name suggests) or forward the e-mail that is
irrelevant (Welcome to the list).
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Since commit be0782e1 we are parsing /proc/meminfo to find out the
default huge page size. However, if the host we are running at does
not support any huge pages (e.g. CONFIG_HUGETLB_PAGE is turned off),
we will not successfully parse the meminfo file and hence the whole
qemu driver init process fails. Moreover, the default huge page size
is needed if and only if there's at least one hugetlbfs mount point.
So the fix consists of moving the virFileGetDefaultHugepageSize
function call after the first hugetlbfs mount point is found.
With this fix, we fail to start with one or more hugetlbfs mounts and
malformed meminfo file, but that's expected (how can one mount
hugetlbfs without kernel supporting huge pages?). Workaround in that
case is to umount all the hugetlbfs mounts.
Reported-by: Jim Fehlig
Signed-off-by: Michal Privoznik
---
src/util/virfile.c | 9 +
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index 9863fd0..f9efc65 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2990,10 +2990,7 @@ virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs,
char mntbuf[1024];
virHugeTLBFSPtr fs = NULL;
size_t nfs = 0;
-unsigned long long default_hugepagesz;
-
-if (virFileGetDefaultHugepageSize(&default_hugepagesz) < 0)
-goto cleanup;
+unsigned long long default_hugepagesz = 0;
if (!(f = setmntent(PROC_MOUNTS, "r"))) {
virReportSystemError(errno,
@@ -3019,6 +3016,10 @@ virFileFindHugeTLBFS(virHugeTLBFSPtr *ret_fs,
if (virFileGetHugepageSize(tmp->mnt_dir, &tmp->size) < 0)
goto cleanup;
+if (!default_hugepagesz &&
+virFileGetDefaultHugepageSize(&default_hugepagesz) < 0)
+goto cleanup;
+
tmp->deflt = tmp->size == default_hugepagesz;
}
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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 and the default path can be
compiled in via --with-qemu-nvram-file configure option.
Signed-off-by: Michal Privoznik
---
configure.ac | 27
libvirt.spec.in| 2 +
src/Makefile.am| 1 +
src/qemu/libvirtd_qemu.aug | 3 +
src/qemu/qemu.conf | 9 +++
src/qemu/qemu_conf.c | 8 +++
src/qemu/qemu_conf.h | 3 +
src/qemu/qemu_process.c| 125 +
src/qemu/test_libvirtd_qemu.aug.in | 1 +
9 files changed, 179 insertions(+)
diff --git a/configure.ac b/configure.ac
index 9dd07d2..16ca266 100644
--- a/configure.ac
+++ b/configure.ac
@@ -569,6 +569,11 @@ AC_ARG_WITH([chrdev-lock-files],
[location for UUCP style lock files for character devices
(use auto for default paths on some platforms) @<:@default=auto@:>@])])
m4_divert_text([DEFAULTS], [with_chrdev_lock_files=auto])
+AC_ARG_WITH([qemu-nvram-file],
+ [AS_HELP_STRING([--with-qemu-nvram-file],
+[location of OVMF_VARS.fd file
+ (use auto for default path on some platforms) @<:@default=auto@:>@])])
+m4_divert_text([DEFAULTS], [with_qemu_nvram_file=auto])
AC_ARG_WITH([pm-utils],
[AS_HELP_STRING([--with-pm-utils],
[use pm-utils for power management @<:@default=yes@:>@])])
@@ -1418,6 +1423,27 @@ platform])
fi
AM_CONDITIONAL([VIR_CHRDEV_LOCK_FILE_PATH], [test "$with_chrdev_lock_files" !=
"no"])
+dnl OVMF_VARS.fd file
+if test "$with_qemu_nvram_file" != "no"; then
+ case $with_qemu_nvram_file in
+ yes | auto)
+dnl Default locations for platforms, or disable if unknown
+if test "$with_linux" = "yes"; then
+ with_qemu_nvram_file=/usr/share/OVMF/OVMF_VARS.fd
+elif test "$with_chrdev_lock_files" = "auto"; then
+ with_qemu_nvram_file=no
+fi ;;
+ esac
+ if test "$with_qemu_nvram_file" = "yes"; then
+AC_MSG_ERROR([You must specify path for the lock files on this
+platform])
+ fi
+ if test "$with_qemu_nvram_file" != "no"; then
+AC_DEFINE_UNQUOTED([VIR_QEMU_NVRAM_FILE_PATH], "$with_qemu_nvram_file",
+ [default path to OVMF_VARS.fd file])
+ fi
+fi
+AM_CONDITIONAL([VIR_QEMU_NVRAM_FILE_PATH], [test "$with_qemu_nvram_file" !=
"no"])
AC_ARG_WITH([secdriver-selinux],
[AS_HELP_STRING([--with-secdriver-selinux],
@@ -2925,6 +2951,7 @@ AC_MSG_NOTICE([numad: $with_numad])
AC_MSG_NOTICE([ XML Catalog: $XML_CATALOG_FILE])
AC_MSG_NOTICE([ Init script: $with_init_script])
AC_MSG_NOTICE([Char device locks: $with_chrdev_lock_files])
+AC_MSG_NOTICE([ OVMF_VARS.fd loc: $with_qemu_nvram_file])
AC_MSG_NOTICE([])
AC_MSG_NOTICE([Developer Tools])
AC_MSG_NOTICE([])
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 29da071..486fc66 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1947,6 +1947,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
@@ -2049,6 +2050,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
diff --git a/src/Makefile.am b/src/Makefile.am
index 982f63d..9bd38f5 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2632,6 +2632,7 @@ endif WITH_SANLOCK
if WITH_QEMU
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/channel/target"
On 08.08.2014 14:08, Paolo Bonzini wrote:
Il 08/08/2014 12:17, Michal Privoznik ha scritto:
+if (loader->nvram) {
+virBufferFreeAndReset(&buf);
+virBufferAsprintf(&buf,
+ "file=%s,if=pflash,format=raw,unit=1",
+ loader->nvram);
+
+virCommandAddArg(cmd, "-drive");
+virCommandAddArgBuffer(cmd, &buf);
+}
Note that other machines may not need unit=1, for example pseries
doesn't need it (it uses -bios for the firmware, not -drive if=pflash).
It would be nice to make this easily configurable.
Is there a table anywhere where I can see what machine types support
this? Or does it makes sense to enable this only for
qemu-system-{x86_64,i386} -M pc and error out for all other combinations?
Alternatively you could use unit=1 if there is a
element, and unit=0 otherwise. We can then patch QEMU to reject unit=1
on machines that use -bios + -drive if=pflash,unit=0.
Laszlo, what happens on x86 with -bios + -drive if=pflash,unit=0?
Paolo
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 10.08.2014 11:44, Guido Günther wrote:
This fixes compilation on kFreeBSD which otherwise fails like
CC util/libvirt_util_la-virprocess.lo
In file included from /usr/include/sys/cpuset.h:35:0,
from util/virprocess.c:43:
/usr/include/sys/_cpuset.h:49:43: error: 'NBBY' undeclared here (not in
a function)
long __bits[howmany(CPU_SETSIZE, _NCPUBITS)];
^
In file included from util/virprocess.c:43:0:
/usr/include/sys/cpuset.h:215:12: error: unknown type name 'cpusetid_t'
int cpuset(cpusetid_t *);
^
/usr/include/sys/cpuset.h:216:30: error: expected ')' before 'id_t'
int cpuset_setid(cpuwhich_t, id_t, cpusetid_t);
^
/usr/include/sys/cpuset.h:217:42: error: expected ')' before 'id_t'
int cpuset_getid(cpulevel_t, cpuwhich_t, id_t, cpusetid_t *);
^
/usr/include/sys/cpuset.h:218:48: error: expected ')' before 'id_t'
int cpuset_getaffinity(cpulevel_t, cpuwhich_t, id_t, size_t, cpuset_t
*);
^
/usr/include/sys/cpuset.h:219:48: error: expected ')' before 'id_t'
int cpuset_setaffinity(cpulevel_t, cpuwhich_t, id_t, size_t, const
cpuset_t *);
And it's the correct usage as documented in
http://www.freebsd.org/cgi/man.cgi?query=cpuset_setid
---
src/util/virprocess.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 9179d73..4d6c50d 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -40,6 +40,7 @@
#endif
#ifdef HAVE_BSD_CPU_AFFINITY
+# include
# include
#endif
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 10.08.2014 13:51, Guido Günther wrote:
Otherwise we fail like
libvirt version: 1.2.7, package: 6 (root 2014-08-08-16:09:22 bogon)
virAuditOpen:62 : Unable to initialize audit layer: Protocol not supported
virFileGetDefaultHugepageSize:2958 : internal error: Unable to parse
/proc/meminfo
virStateInitialize:749 : Initialization of QEMU state driver failed:
internal error: Unable to parse /proc/meminfo
daemonRunStateInit:922 : Driver state initialization failed
if the data can't be determined.
Reference: http://bugs.debian.org/757609
---
src/util/virfile.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/util/virfile.c b/src/util/virfile.c
index f9efc65..b6f5e3f 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2953,8 +2953,9 @@ virFileGetDefaultHugepageSize(unsigned long long *size)
goto cleanup;
if (!(c = strstr(meminfo, HUGEPAGESIZE_STR))) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Unable to parse %s"),
+virReportError(VIR_ERR_NO_SUPPORT,
+ _("%s not found in %s"),
+ HUGEPAGESIZE_STR,
PROC_MEMINFO);
goto cleanup;
}
This merely changes the error code and error message. But the
initialization will fail anyway. Well, it would up till d26e81083. But
the error message you suggests is more verbose and describes the origin
of fault more accurately.
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11.08.2014 11:16, Daniel P. Berrange wrote:
On Tue, Aug 05, 2014 at 11:16:53AM -0600, Jim Fehlig wrote:
Jim Fehlig wrote:
Until recently, the qemu driver has always succeeded in loading even on
a system booted to Xen. Commit be0782e1 broke this, hence libvirtd no
longer loads
xendom0 # /usr/sbin/libvirtd -l
2014-08-04 22:27:31.586+: 35859: info : libvirt version: 1.2.8
2014-08-04 22:27:31.586+: 35859: error :
virFileGetDefaultHugepageSize:2958 : internal error: Unable to parse
/proc/meminfo
2014-08-04 22:27:31.586+: 35859: error : virStateInitialize:749 :
Initialization of QEMU state driver failed: internal error: Unable to
parse /proc/meminfo
2014-08-04 22:27:31.586+: 35859: error : daemonRunStateInit:922 :
Driver state initialization failed
Problem is the Xen kernel does not provide any huge page info in
/proc/meminfo
xendom0 # cat /proc/meminfo | grep -i huge
xendom0 #
BTW, I think this would be the case for any kernel with
CONFIG_HUGETLB_PAGE turned off.
Yes, we should be robust to this situation.
And as of d26e810838fad we are fault tolerant.
Michal, short of always building with '--without-qemu' on a Xen system,
is it possible to make detection of huge pages less fatal wrt loading
the qemu driver?
E.g. assume zero/default for entries not present?
If the dirs / files don't exist, we should just not report any huge
page info at all.
Regards,
Daniel
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11.08.2014 08:41, Jianwei Hu wrote:
qemu: To clear bandwidth settings for a shutoff domain by using domiftune.
After applying this patch, we can use virsh domiftune command to clear inbound
or/and outbound setting for a shutoff domain.
for example:
virsh domiftune $domain $interface 0 0
Thanks for catching this.
Please refer to below virsh help message:
man virsh:
To clear inbound or outbound settings, use --inbound or --outbound respectfully
with average value of zero.
---
src/qemu/qemu_driver.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82a82aa..7db2e9c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9983,11 +9983,17 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
VIR_FREE(persistentNet->bandwidth->in);
persistentNet->bandwidth->in = bandwidth->in;
bandwidth->in = NULL;
+} else {
+VIR_FREE(persistentNet->bandwidth->in);
+persistentNet->bandwidth->in = 0;
We like NULL for pointer more than 0. Moreover, there's no need to
explicitly set pointer freed to NULL as the VIR_FREE() macro does that
already for you (in fact virFree() function does that, whatever).
}
if (bandwidth->out) {
VIR_FREE(persistentNet->bandwidth->out);
persistentNet->bandwidth->out = bandwidth->out;
bandwidth->out = NULL;
+} else {
+VIR_FREE(persistentNet->bandwidth->out);
+persistentNet->bandwidth->out = 0;
}
}
But the fix isn't quite right. For instance:
virsh # domiftune dummy 52:54:00:89:3a:c2 --config
inbound.average: 10
inbound.peak : 0
inbound.burst : 0
outbound.average: 10
outbound.peak : 0
outbound.burst : 0
virsh # domiftune dummy 52:54:00:89:3a:c2 --config 100
virsh # domiftune dummy 52:54:00:89:3a:c2 --config
inbound.average: 100
inbound.peak : 0
inbound.burst : 0
outbound.average: 0
outbound.peak : 0
outbound.burst : 0
The bandwidth is cleared unconditionally. What we really need is:
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 82a82aa..2c3f179 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9983,11 +9983,15 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
VIR_FREE(persistentNet->bandwidth->in);
persistentNet->bandwidth->in = bandwidth->in;
bandwidth->in = NULL;
+} else if (inboundSpecified) {
+VIR_FREE(persistentNet->bandwidth->in);
}
if (bandwidth->out) {
VIR_FREE(persistentNet->bandwidth->out);
persistentNet->bandwidth->out = bandwidth->out;
bandwidth->out = NULL;
+} else if (outboundSpecified) {
+VIR_FREE(persistentNet->bandwidth->out);
}
}
I'm fixing this patch though and pushing. ACK.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11.08.2014 14:59, Wang Rui wrote:
From: Zhou Yimin
If hypervisor is not Xen, the errs in struct _virDomainBlockStats will be -1.
But in KVM when we call domain.blockStats(), errs is 18446744073709551615.
To fix that, this patch has two changes:
1. Replace use of the PyLong_FromUnsignedLongLong with PyLong_FromLongLong
in function libvirt_longlongWrap
2. If the paramemter of libvirt_longlongWrap is unsigned long long,
use libvirt_ulonglongWrap instead because of above change.
After this patch, errs is -1 which is consistent with virDomainBlockStats api.
Signed-off-by: Zhou Yimin
Signed-off-by: Wang Rui
---
libvirt-override.c | 18 +-
typewrappers.c | 2 +-
2 files changed, 10 insertions(+), 10 deletions(-)
ACKed & pushed. Congratulations on your first libvirt-python commit!
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 11.08.2014 12:06, Taowei wrote:
This series of patches rewrite the vbox's domain
driver. The driver is separated into two parts: the version
specified and the common part. The common driver use
vboxUniformedAPI to build a general driver for all vbox
versions. The vboxUniformedAPI take the responsiblity to
communicate with virtualbox. Since there are some incompatible
changes in virtualbox, vboxUniformedAPI should be aware of
these changes and provide a uniformed api for the upper layer.
The significant result of this patch is that we replace all
vir${vbox_version}Driver into one virCommonDriver. So, we will
have only one vbox driver implementation for all vbox versions
in libvirt.
PS: I have send part of my patches before:
https://www.redhat.com/archives/libvir-list/2014-July/msg00937.html
But I have to resend it beacuse I did some improvement on previous
patches:
*Remove the test case for vboxUniformedAPI, because it would raise
"break strict-aliasing rules" warning in some distibutions
*Merged the flag fdWatchNeedInitialize into domainEventCallbacks,
So, we use one flag to indicate whether vbox support callbacks
as well as we need to initialize variables for it.
Taowei (66):
vbox: Begin to rewrite, vboxConnectOpen
vbox: Rewrite vboxConnectClose
vbox: Rewrite vboxDomainSave
vbox: Rewrite vboxConnectGetVersion
vbox: Rewrite vboxConnectGetHostname
vbox: Rewrite vboxConnectIsSecure
vbox: Rewrite vboxConnectIsEncrypted
vbox: Rewrite vboxConnectIsAlive
vbox: Rewrite vboxConnectGetMaxVcpus
vbox: Rewrite vboxConnectGetCapabilities
vbox: Rewrite vboxConnectListDomains
vbox: Rewrite vboxConnectNumOfDomains
vbox: Rewrite vboxDomainLookupById
vbox: Rewrite vboxDomainLookupByUUID
vbox: Rewrite vboxDomainUndefineFlags
vbox: Rewrite vboxDomainDefineXML
vbox: Rewrite vboxDomainCreateWithFlags
vbox: Rewrite vboxDomainCreate
vbox: Rewrite vboxDomainCreateXML
vbox: Rewrite vboxDomainLookupByName
vbox: Rewrite vboxDomainIsActive
vbox: Rewrite vboxDomainIsPersistent
vbox: Rewrite vboxDomainIsUpdated
vbox: Rewrite vboxDomainSuspend
vbox: Rewrite vboxDomainResume
vbox: Rewrite vboxDomainShutdownFlags
vbox: Rewrite vboxDomainShutdown
vbox: Rewrite vboxDomainReboot
vbox: Rewrite vboxDomainDestroyFlags
vbox: Rewrite vboxDomainDestroy
vbox: Rewrite vboxDomainGetOSType
vbox: Rewrite vboxDomainSetMemory
vbox: Rewrite vboxDomainGetInfo
vbox: Rewrite vboxDomainGetState
vbox: Rewrite vboxDomainSetVcpusFlags
vbox: Rewrite vboxDomainSetVcpus
vbox: Rewrite vboxDomainGetVcpusFlags
vbox: Rewrite vboxDomainGetMaxVcpus
vbox: Add API for vboxDomainGetXMLDesc
vbox: Rewrite vboxDomainGetXMLDesc
vbox: Rewrite vboxConnectListDefinedDomains
vbox: Rewrite vboxConnectNumOfDefinedDomains
vbox: Rewrite vboxDomainUndefine
vbox: Rewrite vboxDomainAttachDevice
vbox: Rewrite vboxDomainAttachDeviceFlags
vbox: Rewrite vboxDomainUpdateDeviceFlags
vbox: Rewrite vboxDomainDetachDevice
vbox: Rewrite vboxDomainDetachDeviceFlags
vbox: Add API for vboxDomainSnapshotCreateXML
vbox: Rewrite vboxDomainSnapshotCreateXML
vbox: Rewrite vboxDomainSnapshotGetXMLDesc
vbox: Rewrite vboxDomainSnapshotNum
vbox: Rewrite vboxDomainSnapshotListNames
vbox: Rewrite vboxSnapshotLookupByName
vbox: Rewrite vboxDomainHasCurrentSnapshot
vbox: Rewrite vboxDomainSnapshotGetParent
vbox: Rewrite vboxDomainSnapshotCurrent
vbox: Rewrite vboxDomainSnapshotIsCurrent
vbox: Rewrite vboxDomainSnapshotHasMetadata
vbox: Rewrite vboxDomainRevertToSnapshot
vbox: Rewrite vboxDomainSnapshotDelete
vbox: Rewrite vboxDomainScreenshot
vbox: Rewrite vboxConnectListAllDomains
vbox: Rewrite vboxNode functions
vbox: Add registerDomainEvent
vbox: Introducing vboxCommonDriver
po/POTFILES.in|1 +
src/Makefile.am |5 +-
src/vbox/README |7 +-
src/vbox/vbox_common.c| 7550 +
src/vbox/vbox_common.h| 306 +
src/vbox/vbox_driver.c| 40 +-
src/vbox/vbox_install_api.h | 26 +
src/vbox/vbox_tmpl.c |14557 +
src/vbox/vbox_uniformed_api.h | 551 ++
9 files changed, 13186 insertions(+), 9857 deletions(-)
create mode 100644 src/vbox/vbox_common.c
create mode 100644 src/vbox/vbox_common.h
create mode 100644 src/vbox/vbox_install_api.h
create mode 100644 src/vbox/vbox_uniformed_api.h
ACK to all the patches. I've fixed all the small nits I found. I'm
keeping the patches on my private branch for some time to give others
time to share their opinions. Nevertheless, incredible work in making
the vbox driver look more sane what you've done!
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
At the beginning of the qemu config file parsing function there
are 3 helper macros defined: GET_VALUE_BOOL, GET_VALUE_LONG and
GET_VALUE_STR. Later, when they are no longer needed they are
undefined in order to keep the namespace clean. However, the
GET_VALUE_STRING is undefined instead of GET_VALUE_STR.
Signed-off-by: Michal Privoznik
---
Pushed as trivial.
src/qemu/qemu_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index b14b1bc..238d2b1 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -662,7 +662,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg,
}
#undef GET_VALUE_BOOL
#undef GET_VALUE_LONG
-#undef GET_VALUE_STRING
+#undef GET_VALUE_STR
virQEMUDriverConfigPtr virQEMUDriverGetConfig(virQEMUDriverPtr driver)
{
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 13.08.2014 00:00, David kiarie wrote:
Hi there,
I get this errors trying to intialialize libvirtd
$libvirtd -vvv
2014-08-12 21:57:47.887+: 6449: info : libvirt version: 1.2.8
2014-08-12 21:57:47.887+: 6449: warning : virGetHostname:665 :
getaddrinfo failed for 'linux-xzc4': Name or service not known
2014-08-12 21:57:48.034+: 6461: info : libvirt version: 1.2.8
2014-08-12 21:57:48.034+: 6461: error :
virFileGetDefaultHugepageSize:2958 : internal error: Unable to parse
/proc/meminfo
2014-08-12 21:57:48.034+: 6461: error : virStateInitialize:749 :
Initialization of QEMU state driver failed: internal error: Unable to
parse /proc/meminfo
2014-08-12 21:57:48.034+: 6461: error : daemonRunStateInit:922 :
Driver state initialization failed
Is someone else experiencing the same thing?
There's been several patches that are merged now that fixes the problem
(e.g. d26e810838fad49). So please try fetching the currnet git HEAD and
build it. The problem should be fixed already.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 12.08.2014 21:09, Sean Noonan wrote:
> We're using sasl+gssapi+kerberos to do authentication for libvirt,
> including from hypervisor to hypervisor. However, the environmental
> variable filtering implemented in libvirt prevents this from working, so
> we're forced to run a locally patched version.
>
> Thoughts on the following patch to pass the location of the local
> credential cache as well?
>
> --- a/src/util/vircommand.c2014-01-07 14:14:11.388934108 +
> +++ b/src/util/vircommand.c2014-01-07 14:18:14.725082505 +
> @@ -1314,6 +1314,7 @@
>
> virCommandAddEnvPair(cmd, "LC_ALL", "C");
>
> +virCommandAddEnvPassBlockSUID(cmd, "KRB5CCNAME", NULL);
> virCommandAddEnvPassBlockSUID(cmd, "LD_PRELOAD", NULL);
> virCommandAddEnvPassBlockSUID(cmd, "LD_LIBRARY_PATH", NULL);
> virCommandAddEnvPassBlockSUID(cmd, "PATH", "/bin:/usr/bin");
I've got some doubts whether this is the correct approach. This will pass the
environment variable to every command spawned. Do we really want every command
have access to kerberos tickets? On the other hand, we've done this for a
limited use case:
commit afc984af2e055b33f7c7b06ccacaf00103d4ce5e
Author: Michal Privoznik
AuthorDate: Fri Sep 9 15:59:26 2011 +0200
Commit: Michal Privoznik
CommitDate: Fri Sep 9 15:59:26 2011 +0200
virnetsocket: Pass KRB5CCNAME env variable
So we can allow GSSAPI authentication for ssh.
Signed-off-by: Matthias Witte
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 275b70d..88dc2a4 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -615,6 +615,7 @@ int virNetSocketNewConnectSSH(const char *nodename,
cmd = virCommandNew(binary ? binary : "ssh");
virCommandAddEnvPassCommon(cmd);
+virCommandAddEnvPass(cmd, "KRB5CCNAME");
virCommandAddEnvPass(cmd, "SSH_AUTH_SOCK");
virCommandAddEnvPass(cmd, "SSH_ASKPASS");
virCommandAddEnvPass(cmd, "DISPLAY");
Moreover, is KRB5CCNAME enough? What about KRB5_KTNAME?
And regarding the patch itself, I think you want to increase the preallocation
constant too:
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index e775ba6..aa493cd 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -1312,7 +1312,7 @@ virCommandAddEnvPassCommon(virCommandPtr cmd)
if (!cmd || cmd->has_error)
return;
-if (VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 9) < 0) {
+if (VIR_RESIZE_N(cmd->env, cmd->maxenv, cmd->nenv, 10) < 0) {
cmd->has_error = ENOMEM;
return;
}
@@ -1326,6 +1326,7 @@ virCommandAddEnvPassCommon(virCommandPtr cmd)
virCommandAddEnvPassAllowSUID(cmd, "USER");
virCommandAddEnvPassAllowSUID(cmd, "LOGNAME");
virCommandAddEnvPassBlockSUID(cmd, "TMPDIR", NULL);
+virCommandAddEnvPassBlockSUID(cmd, "KRB5CCNAME", NULL);
}
/**
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
An advice appeared there on the qemu-devel list [1]. When a domain is
suspended and then resumed guest kernel is not aware of this. So we've
introduced virDomainSetTime API that resets the time within guest
using qemu-ga. On the other hand, qemu itself is trying to make RTC
beat faster to catch the difference. But if we don't tell qemu that
guest's time was reset via the other method, both mechanisms are
applied resulting in again wrong guest time. In order to avoid summing
both corrections we need to tell qemu that it should not use the RTC
injection if the guest time is set via guest agent.
1: http://www.mail-archive.com/qemu-devel@nongnu.org/msg236435.html
Signed-off-by: Michal Privoznik
---
src/qemu/qemu_driver.c | 10 ++
src/qemu/qemu_monitor.c | 33 +
src/qemu/qemu_monitor.h | 2 ++
src/qemu/qemu_monitor_json.c | 21 +
src/qemu/qemu_monitor_json.h | 2 ++
5 files changed, 68 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b6219ba..bdfd155 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16879,6 +16879,16 @@ qemuDomainSetTime(virDomainPtr dom,
rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync);
qemuDomainObjExitAgent(vm);
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+goto endjob;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv->mon);
+qemuDomainObjExitMonitor(driver, vm);
+
if (rv < 0)
goto endjob;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 3d9f87b..77627bc 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4037,3 +4037,36 @@ qemuMonitorGetGuestCPU(qemuMonitorPtr mon,
return qemuMonitorJSONGetGuestCPU(mon, arch, data);
}
+
+/**
+ * qemuMonitorRTCResetReinjection:
+ * @mon: Pointer to the monitor
+ *
+ * Issue rtc-reset-reinjection command.
+ * This should be used in cases where guest time is restored via
+ * guest agent so RTC injection is not needed (in fact it will
+ * confuse guest's RTC).
+ *
+ * Returns 0 on success
+ *-1 on error.
+ */
+int
+qemuMonitorRTCResetReinjection(qemuMonitorPtr mon)
+{
+
+VIR_DEBUG("mon=%p", mon);
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+return -1;
+}
+
+if (!mon->json) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("JSON monitor is required"));
+return -1;
+}
+
+return qemuMonitorJSONRTCResetReinjection(mon);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index c3695f2..4fd6f01 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -790,6 +790,8 @@ int qemuMonitorGetGuestCPU(qemuMonitorPtr mon,
virArch arch,
virCPUDataPtr *data);
+int qemuMonitorRTCResetReinjection(qemuMonitorPtr mon);
+
/**
* When running two dd process and using <> redirection, we need a
* shell that will not truncate files. These two strings serve that
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a62c02f..538110c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5851,3 +5851,24 @@ qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon,
return -1;
}
}
+
+int
+qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("rtc-reset-reinjection",
+ NULL)))
+return ret;
+
+ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+if (ret == 0)
+ret = qemuMonitorJSONCheckError(cmd, reply);
+
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 5f6c846..d8c9308 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -437,4 +437,6 @@ int qemuMonitorJSONGetDeviceAliases(qemuMonitorPtr mon,
int qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon,
virArch arch,
virCPUDataPtr *data);
+
+int qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon);
#endif /* QEMU_MONITOR_JSON_H */
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[CC-ing Yohan BELLEGUIC and Manuel VIVES]
On 12.08.2014 17:31, Michal Privoznik wrote:
On 11.08.2014 12:06, Taowei wrote:
This series of patches rewrite the vbox's domain
driver. The driver is separated into two parts: the version
specified and the common part. The common driver use
vboxUniformedAPI to build a general driver for all vbox
versions. The vboxUniformedAPI take the responsiblity to
communicate with virtualbox. Since there are some incompatible
changes in virtualbox, vboxUniformedAPI should be aware of
these changes and provide a uniformed api for the upper layer.
The significant result of this patch is that we replace all
vir${vbox_version}Driver into one virCommonDriver. So, we will
have only one vbox driver implementation for all vbox versions
in libvirt.
PS: I have send part of my patches before:
https://www.redhat.com/archives/libvir-list/2014-July/msg00937.html
But I have to resend it beacuse I did some improvement on previous
patches:
*Remove the test case for vboxUniformedAPI, because it would raise
"break strict-aliasing rules" warning in some distibutions
*Merged the flag fdWatchNeedInitialize into domainEventCallbacks,
So, we use one flag to indicate whether vbox support callbacks
as well as we need to initialize variables for it.
Taowei (66):
vbox: Begin to rewrite, vboxConnectOpen
vbox: Rewrite vboxConnectClose
vbox: Rewrite vboxDomainSave
vbox: Rewrite vboxConnectGetVersion
vbox: Rewrite vboxConnectGetHostname
vbox: Rewrite vboxConnectIsSecure
vbox: Rewrite vboxConnectIsEncrypted
vbox: Rewrite vboxConnectIsAlive
vbox: Rewrite vboxConnectGetMaxVcpus
vbox: Rewrite vboxConnectGetCapabilities
vbox: Rewrite vboxConnectListDomains
vbox: Rewrite vboxConnectNumOfDomains
vbox: Rewrite vboxDomainLookupById
vbox: Rewrite vboxDomainLookupByUUID
vbox: Rewrite vboxDomainUndefineFlags
vbox: Rewrite vboxDomainDefineXML
vbox: Rewrite vboxDomainCreateWithFlags
vbox: Rewrite vboxDomainCreate
vbox: Rewrite vboxDomainCreateXML
vbox: Rewrite vboxDomainLookupByName
vbox: Rewrite vboxDomainIsActive
vbox: Rewrite vboxDomainIsPersistent
vbox: Rewrite vboxDomainIsUpdated
vbox: Rewrite vboxDomainSuspend
vbox: Rewrite vboxDomainResume
vbox: Rewrite vboxDomainShutdownFlags
vbox: Rewrite vboxDomainShutdown
vbox: Rewrite vboxDomainReboot
vbox: Rewrite vboxDomainDestroyFlags
vbox: Rewrite vboxDomainDestroy
vbox: Rewrite vboxDomainGetOSType
vbox: Rewrite vboxDomainSetMemory
vbox: Rewrite vboxDomainGetInfo
vbox: Rewrite vboxDomainGetState
vbox: Rewrite vboxDomainSetVcpusFlags
vbox: Rewrite vboxDomainSetVcpus
vbox: Rewrite vboxDomainGetVcpusFlags
vbox: Rewrite vboxDomainGetMaxVcpus
vbox: Add API for vboxDomainGetXMLDesc
vbox: Rewrite vboxDomainGetXMLDesc
vbox: Rewrite vboxConnectListDefinedDomains
vbox: Rewrite vboxConnectNumOfDefinedDomains
vbox: Rewrite vboxDomainUndefine
vbox: Rewrite vboxDomainAttachDevice
vbox: Rewrite vboxDomainAttachDeviceFlags
vbox: Rewrite vboxDomainUpdateDeviceFlags
vbox: Rewrite vboxDomainDetachDevice
vbox: Rewrite vboxDomainDetachDeviceFlags
vbox: Add API for vboxDomainSnapshotCreateXML
vbox: Rewrite vboxDomainSnapshotCreateXML
vbox: Rewrite vboxDomainSnapshotGetXMLDesc
vbox: Rewrite vboxDomainSnapshotNum
vbox: Rewrite vboxDomainSnapshotListNames
vbox: Rewrite vboxSnapshotLookupByName
vbox: Rewrite vboxDomainHasCurrentSnapshot
vbox: Rewrite vboxDomainSnapshotGetParent
vbox: Rewrite vboxDomainSnapshotCurrent
vbox: Rewrite vboxDomainSnapshotIsCurrent
vbox: Rewrite vboxDomainSnapshotHasMetadata
vbox: Rewrite vboxDomainRevertToSnapshot
vbox: Rewrite vboxDomainSnapshotDelete
vbox: Rewrite vboxDomainScreenshot
vbox: Rewrite vboxConnectListAllDomains
vbox: Rewrite vboxNode functions
vbox: Add registerDomainEvent
vbox: Introducing vboxCommonDriver
po/POTFILES.in|1 +
src/Makefile.am |5 +-
src/vbox/README |7 +-
src/vbox/vbox_common.c| 7550 +
src/vbox/vbox_common.h| 306 +
src/vbox/vbox_driver.c| 40 +-
src/vbox/vbox_install_api.h | 26 +
src/vbox/vbox_tmpl.c |14557
+
src/vbox/vbox_uniformed_api.h | 551 ++
9 files changed, 13186 insertions(+), 9857 deletions(-)
create mode 100644 src/vbox/vbox_common.c
create mode 100644 src/vbox/vbox_common.h
create mode 100644 src/vbox/vbox_install_api.h
create mode 100644 src/vbox/vbox_uniformed_api.h
ACK to all the patches. I've fixed all the small nits I found. I'm
keeping the patches on my private branch for some time to give others
time to share their opinions. Nevertheless, incredible work in making
the vbox driver look more sane what you've done!
Yohan and Manuel,
can you guys please give this patchset a test? You seem to be interes
On 13.08.2014 18:30, Eric Blake wrote:
On 08/13/2014 06:51 AM, Michal Privoznik wrote:
s/_/-/2 in the subject line
An advice appeared there on the qemu-devel list [1]. When a domain is
suspended and then resumed guest kernel is not aware of this. So we've
introduced virDomainSetTime API that resets the time within guest
using qemu-ga. On the other hand, qemu itself is trying to make RTC
beat faster to catch the difference. But if we don't tell qemu that
guest's time was reset via the other method, both mechanisms are
applied resulting in again wrong guest time. In order to avoid summing
both corrections we need to tell qemu that it should not use the RTC
injection if the guest time is set via guest agent.
1: http://www.mail-archive.com/qemu-devel@nongnu.org/msg236435.html
Signed-off-by: Michal Privoznik
---
+++ b/src/qemu/qemu_driver.c
@@ -16879,6 +16879,16 @@ qemuDomainSetTime(virDomainPtr dom,
rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync);
qemuDomainObjExitAgent(vm);
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+goto endjob;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv->mon);
+qemuDomainObjExitMonitor(driver, vm);
This forces the command to fail if qemu is too old to have
rtc-reset-reinjection but the agent is new enough to set time. Should
you make this code conditional on whether qemu supports the QMP command?
I'm not sure. If that's the case, both corrections will apply so guest
ends up with incorrect time anyway. And if the API is to guarantee
correctly set time, it must fail if such guarantees can't be made IMO.
}
+
+int
+qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("rtc-reset-reinjection",
+ NULL)))
+return ret;
+
+ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+if (ret == 0)
+ret = qemuMonitorJSONCheckError(cmd, reply);
+
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
Is it worth enhancing the testsuite to add coverage for this command and
expected response?
Yeah. I'll post v2.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
https://bugzilla.redhat.com/show_bug.cgi?id=1103245
An advice appeared there on the qemu-devel list [1]. When a domain is
suspended and then resumed guest kernel is not aware of this. So we've
introduced virDomainSetTime API that resets the time within guest
using qemu-ga. On the other hand, qemu itself is trying to make RTC
beat faster to catch the difference. But if we don't tell qemu that
guest's time was reset via the other method, both mechanisms are
applied resulting in again wrong guest time. In order to avoid summing
both corrections we need to tell qemu that it should not use the RTC
injection if the guest time is set via guest agent.
1: http://www.mail-archive.com/qemu-devel@nongnu.org/msg236435.html
Signed-off-by: Michal Privoznik
---
Notes:
diff to v1:
-fixed command name in subject
-added testcase
src/qemu/qemu_driver.c | 10 ++
src/qemu/qemu_monitor.c | 33 +
src/qemu/qemu_monitor.h | 2 ++
src/qemu/qemu_monitor_json.c | 21 +
src/qemu/qemu_monitor_json.h | 2 ++
tests/qemumonitorjsontest.c | 1 +
6 files changed, 69 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b6219ba..bdfd155 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -16879,6 +16879,16 @@ qemuDomainSetTime(virDomainPtr dom,
rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync);
qemuDomainObjExitAgent(vm);
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+goto endjob;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv->mon);
+qemuDomainObjExitMonitor(driver, vm);
+
if (rv < 0)
goto endjob;
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 3d9f87b..77627bc 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -4037,3 +4037,36 @@ qemuMonitorGetGuestCPU(qemuMonitorPtr mon,
return qemuMonitorJSONGetGuestCPU(mon, arch, data);
}
+
+/**
+ * qemuMonitorRTCResetReinjection:
+ * @mon: Pointer to the monitor
+ *
+ * Issue rtc-reset-reinjection command.
+ * This should be used in cases where guest time is restored via
+ * guest agent so RTC injection is not needed (in fact it will
+ * confuse guest's RTC).
+ *
+ * Returns 0 on success
+ *-1 on error.
+ */
+int
+qemuMonitorRTCResetReinjection(qemuMonitorPtr mon)
+{
+
+VIR_DEBUG("mon=%p", mon);
+
+if (!mon) {
+virReportError(VIR_ERR_INVALID_ARG, "%s",
+ _("monitor must not be NULL"));
+return -1;
+}
+
+if (!mon->json) {
+virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("JSON monitor is required"));
+return -1;
+}
+
+return qemuMonitorJSONRTCResetReinjection(mon);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index c3695f2..4fd6f01 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -790,6 +790,8 @@ int qemuMonitorGetGuestCPU(qemuMonitorPtr mon,
virArch arch,
virCPUDataPtr *data);
+int qemuMonitorRTCResetReinjection(qemuMonitorPtr mon);
+
/**
* When running two dd process and using <> redirection, we need a
* shell that will not truncate files. These two strings serve that
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index a62c02f..538110c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -5851,3 +5851,24 @@ qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon,
return -1;
}
}
+
+int
+qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon)
+{
+int ret = -1;
+virJSONValuePtr cmd;
+virJSONValuePtr reply = NULL;
+
+if (!(cmd = qemuMonitorJSONMakeCommand("rtc-reset-reinjection",
+ NULL)))
+return ret;
+
+ret = qemuMonitorJSONCommand(mon, cmd, &reply);
+
+if (ret == 0)
+ret = qemuMonitorJSONCheckError(cmd, reply);
+
+virJSONValueFree(cmd);
+virJSONValueFree(reply);
+return ret;
+}
diff --git a/src/qemu/qemu_monitor_json.h b/src/qemu/qemu_monitor_json.h
index 5f6c846..d8c9308 100644
--- a/src/qemu/qemu_monitor_json.h
+++ b/src/qemu/qemu_monitor_json.h
@@ -437,4 +437,6 @@ int qemuMonitorJSONGetDeviceAliases(qemuMonitorPtr mon,
int qemuMonitorJSONGetGuestCPU(qemuMonitorPtr mon,
virArch arch,
virCPUDataPtr *data);
+
+int qemuMonitorJSONRTCResetReinjection(qemuMonitorPtr mon);
#endif /* QEMU_MONITOR_JSON_H */
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index baee80a..e3fb4f7 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjson
On 14.08.2014 09:55, Wang Rui wrote:
From: Mo yuxiang
commit d9504941 introduces two new attributes "cmd_per_lun" and
"max_sectors" same with the names QEMU uses for virtio-scsi.
But the case of parsing them is not exact. Change to parse
them if controller has "driver" element.
Signed-off-by: Mo yuxiang
---
src/conf/domain_conf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 934f6cb..5c762fa 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -6295,10 +6295,11 @@ virDomainControllerDefParseXML(xmlNodePtr node,
cur = node->children;
while (cur != NULL) {
if (cur->type == XML_ELEMENT_NODE) {
-if (xmlStrEqual(cur->name, BAD_CAST "driver"))
+if (xmlStrEqual(cur->name, BAD_CAST "driver")) {
queues = virXMLPropString(cur, "queues");
cmd_per_lun = virXMLPropString(cur, "cmd_per_lun");
max_sectors = virXMLPropString(cur, "max_sectors");
+}
}
cur = cur->next;
}
ACKed and pushed. Unfortunately, the 1.2.7 release is broken so I'm
pushing this into 1.2.7 maint branch too.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 14.08.2014 05:37, Jincheng Miao wrote:
Perl is necessary to our build processing, it will invoke a lot of
generating script, like: gendispatch.pl. If perl is missing, it's
ok for build from git checkout, because autogen.sh will tell you.
But for compiling from a release tarball, configure will just record
a missing message, and continue, then build failed, like:
https://www.redhat.com/archives/libvirt-users/2014-August/msg00050.html
So need to enhance configure script to handle this negative case.
Reported-by: Hongbin Lu
Signed-off-by: Jincheng Miao
---
configure.ac |3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/configure.ac b/configure.ac
index 081f298..af3fe28 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2173,6 +2173,9 @@ AM_CONDITIONAL([WITH_HYPERV], [test "$with_hyperv" =
"yes"])
dnl Allow perl/python overrides
AC_PATH_PROGS([PYTHON], [python2 python])
AC_PATH_PROG([PERL], [perl])
+if test -z "$PERL"; then
+ AC_MSG_ERROR([Failed to find perl.])
+fi
AC_ARG_WITH([test-suite],
[AS_HELP_STRING([--with-test-suite],
I'm inclined to ACK this. We currently have some files that can be built
from git and are contained in the release so we cut off the set of
required tools to build the libvirt. However, with so widely accessible
tool as perl (which is almost everywhere) we don't need to do that.
Moreover, there are some Makefile targets which have runtime dependency
on perl, e.g. bracket-spacing-check syntax-check rule.
And with this change I think we need this one too:
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 29da071..f491de7 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -446,6 +446,7 @@ BuildRequires: gettext-devel
BuildRequires: libtool
BuildRequires: /usr/bin/pod2man
%endif
+BuildRequires: perl
BuildRequires: python
%if %{with_systemd}
BuildRequires: systemd-units
Again, I'm not expecting to see any RPM based distribution without perl
(esp. if rpm-build package requires perl itself). But we should have it
for completeness.
So I'm squashing it in and pushing.
ACK
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 14.08.2014 10:41, Ján Tomko wrote:
Also add qemuDomainChangeGraphicsPasswords,
qemuProcessVerifyGuestCPU and qemuProcessInitPCIAddresses.
Replace tabs by spaces.
---
I'll do some testing on the patch and push it later with this squashed in
if it works well.
src/qemu/qemu_domain.c | 2 +-
src/qemu/qemu_hotplug.c | 12
src/qemu/qemu_hotplug.h | 3 ++-
src/qemu/qemu_process.c | 33 -
4 files changed, 31 insertions(+), 19 deletions(-)
ACK to the original with this squashed in.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 14.08.2014 17:50, Yohan Belleguic wrote:
Le Wednesday 13 August 2014 17:18:44, Michal Privoznik a écrit :
[CC-ing Yohan BELLEGUIC and Manuel VIVES]
On 12.08.2014 17:31, Michal Privoznik wrote:
On 11.08.2014 12:06, Taowei wrote:
This series of patches rewrite the vbox's domain
driver. The driver is separated into two parts: the version
specified and the common part. The common driver use
vboxUniformedAPI to build a general driver for all vbox
versions. The vboxUniformedAPI take the responsiblity to
communicate with virtualbox. Since there are some incompatible
changes in virtualbox, vboxUniformedAPI should be aware of
these changes and provide a uniformed api for the upper layer.
The significant result of this patch is that we replace all
vir${vbox_version}Driver into one virCommonDriver. So, we will
have only one vbox driver implementation for all vbox versions
in libvirt.
PS: I have send part of my patches before:
https://www.redhat.com/archives/libvir-list/2014-July/msg00937.html
But I have to resend it beacuse I did some improvement on previous
patches:
*Remove the test case for vboxUniformedAPI, because it would raise
"break strict-aliasing rules" warning in some distibutions
*Merged the flag fdWatchNeedInitialize into domainEventCallbacks,
So, we use one flag to indicate whether vbox support callbacks
as well as we need to initialize variables for it.
Taowei (66):
vbox: Begin to rewrite, vboxConnectOpen
vbox: Rewrite vboxConnectClose
vbox: Rewrite vboxDomainSave
vbox: Rewrite vboxConnectGetVersion
vbox: Rewrite vboxConnectGetHostname
vbox: Rewrite vboxConnectIsSecure
vbox: Rewrite vboxConnectIsEncrypted
vbox: Rewrite vboxConnectIsAlive
vbox: Rewrite vboxConnectGetMaxVcpus
vbox: Rewrite vboxConnectGetCapabilities
vbox: Rewrite vboxConnectListDomains
vbox: Rewrite vboxConnectNumOfDomains
vbox: Rewrite vboxDomainLookupById
vbox: Rewrite vboxDomainLookupByUUID
vbox: Rewrite vboxDomainUndefineFlags
vbox: Rewrite vboxDomainDefineXML
vbox: Rewrite vboxDomainCreateWithFlags
vbox: Rewrite vboxDomainCreate
vbox: Rewrite vboxDomainCreateXML
vbox: Rewrite vboxDomainLookupByName
vbox: Rewrite vboxDomainIsActive
vbox: Rewrite vboxDomainIsPersistent
vbox: Rewrite vboxDomainIsUpdated
vbox: Rewrite vboxDomainSuspend
vbox: Rewrite vboxDomainResume
vbox: Rewrite vboxDomainShutdownFlags
vbox: Rewrite vboxDomainShutdown
vbox: Rewrite vboxDomainReboot
vbox: Rewrite vboxDomainDestroyFlags
vbox: Rewrite vboxDomainDestroy
vbox: Rewrite vboxDomainGetOSType
vbox: Rewrite vboxDomainSetMemory
vbox: Rewrite vboxDomainGetInfo
vbox: Rewrite vboxDomainGetState
vbox: Rewrite vboxDomainSetVcpusFlags
vbox: Rewrite vboxDomainSetVcpus
vbox: Rewrite vboxDomainGetVcpusFlags
vbox: Rewrite vboxDomainGetMaxVcpus
vbox: Add API for vboxDomainGetXMLDesc
vbox: Rewrite vboxDomainGetXMLDesc
vbox: Rewrite vboxConnectListDefinedDomains
vbox: Rewrite vboxConnectNumOfDefinedDomains
vbox: Rewrite vboxDomainUndefine
vbox: Rewrite vboxDomainAttachDevice
vbox: Rewrite vboxDomainAttachDeviceFlags
vbox: Rewrite vboxDomainUpdateDeviceFlags
vbox: Rewrite vboxDomainDetachDevice
vbox: Rewrite vboxDomainDetachDeviceFlags
vbox: Add API for vboxDomainSnapshotCreateXML
vbox: Rewrite vboxDomainSnapshotCreateXML
vbox: Rewrite vboxDomainSnapshotGetXMLDesc
vbox: Rewrite vboxDomainSnapshotNum
vbox: Rewrite vboxDomainSnapshotListNames
vbox: Rewrite vboxSnapshotLookupByName
vbox: Rewrite vboxDomainHasCurrentSnapshot
vbox: Rewrite vboxDomainSnapshotGetParent
vbox: Rewrite vboxDomainSnapshotCurrent
vbox: Rewrite vboxDomainSnapshotIsCurrent
vbox: Rewrite vboxDomainSnapshotHasMetadata
vbox: Rewrite vboxDomainRevertToSnapshot
vbox: Rewrite vboxDomainSnapshotDelete
vbox: Rewrite vboxDomainScreenshot
vbox: Rewrite vboxConnectListAllDomains
vbox: Rewrite vboxNode functions
vbox: Add registerDomainEvent
vbox: Introducing vboxCommonDriver
po/POTFILES.in|1 +
src/Makefile.am |5 +-
src/vbox/README |7 +-
src/vbox/vbox_common.c| 7550 +
src/vbox/vbox_common.h| 306 +
src/vbox/vbox_driver.c| 40 +-
src/vbox/vbox_install_api.h | 26 +
src/vbox/vbox_tmpl.c |14557
+
src/vbox/vbox_uniformed_api.h | 551 ++
9 files changed, 13186 insertions(+), 9857 deletions(-)
create mode 100644 src/vbox/vbox_common.c
create mode 100644 src/vbox/vbox_common.h
create mode 100644 src/vbox/vbox_install_api.h
create mode 100644 src/vbox/vbox_uniformed_api.h
ACK to all the patches. I've fixed all the small nits I found. I'm
keeping the patches on my private branch for some time to give others
time to share the
Dear list,
Virtualbox driver has its own implementation of storage and network
sub-drivers. But as of commit ba5f3c7c8ecc1037e44904916989a1c65777a9d5
(contained in the 1.0.6 release) when the VBox moved from client to
daemon, the storage and network sub-drivers are indeed registered but in
fact never called. It's due to our virConnectOpen function where the
general network and storage drivers take precedence. So I guess my
question is: should we drop the VBox sub-drivers or perhaps fix the
virConnectOpen function?
Reported-by: Taowei Luo
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
There are some stateless drivers which implement subdrivers
(typically vbox and its own network and storage subdrivers). However,
as of ba5f3c7c8ecc10 the vbox driver lives in the daemon, not the
client library. This means, in order for vbox (or any stateless domain
driver) to use its subdrivers, it must register before the general
drivers. Later, when the virConnectOpen function goes through the
subdrivers, stateless drivers are searched first. If the connection
request is aiming at stateless driver, it will be opened. Otherwise
the generic subdriver is opened.
The other change done in this commit is moving interface module load a
bit earlier to match the ordering in case libvirt is built without
driver modules.
Reported-by: Taowei Luo
Signed-off-by: Michal Privoznik
---
daemon/libvirtd.c | 31 ++-
1 file changed, 18 insertions(+), 13 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index a1f64ad..69baef6 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -365,10 +365,15 @@ static void daemonInitialize(void)
{
/*
* Note that the order is important: the first ones have a higher
- * priority when calling virStateInitialize. We must register
- * the network, storage and nodedev drivers before any domain
- * drivers, since their resources must be auto-started before
- * any domains can be auto-started.
+ * priority when calling virStateInitialize. We must register the
+ * network, storage and nodedev drivers before any stateless
+ * domain drivers, since their resources must be auto-started
+ * before any domains can be auto-started. Moreover, some
+ * stateless drivers implement their own subdrivers (e.g. the vbox
+ * driver has its own network and storage subdriers) which need to
+ * have higher priority. Otherwise, when connecting the such
+ * driver generic subdriver may be opened instead of the one
+ * corresponding to the stateless driver.
*/
#ifdef WITH_DRIVER_MODULES
/* We don't care if any of these fail, because the whole point
@@ -376,9 +381,15 @@ static void daemonInitialize(void)
* If they try to open a connection for a module that
* is not loaded they'll get a suitable error at that point
*/
+# ifdef WITH_VBOX
+virDriverLoadModule("vbox");
+# endif
# ifdef WITH_NETWORK
virDriverLoadModule("network");
# endif
+# ifdef WITH_INTERFACE
+virDriverLoadModule("interface");
+# endif
# ifdef WITH_STORAGE
virDriverLoadModule("storage");
# endif
@@ -391,9 +402,6 @@ static void daemonInitialize(void)
# ifdef WITH_NWFILTER
virDriverLoadModule("nwfilter");
# endif
-# ifdef WITH_INTERFACE
-virDriverLoadModule("interface");
-# endif
# ifdef WITH_XEN
virDriverLoadModule("xen");
# endif
@@ -409,13 +417,13 @@ 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();
+# endif
# ifdef WITH_NETWORK
networkRegister();
# endif
@@ -449,9 +457,6 @@ static void daemonInitialize(void)
# ifdef WITH_UML
umlRegister();
# endif
-# ifdef WITH_VBOX
-vboxRegister();
-# endif
# ifdef WITH_BHYVE
bhyveRegister();
# endif
--
1.8.5.5
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
---
configure.ac | 1 -
libvirt.spec.in| 2 +
src/Makefile.am| 1 +
src/qemu/libvirtd_qemu.aug | 3 +
src/qemu/qemu.conf | 14
src/qemu/qemu_conf.c | 92 ++
src/qemu/qemu_conf.h | 5 ++
src/qemu/qemu_process.c| 131 +
src/qemu/test_libvirtd_qemu.aug.in | 3 +
9 files changed, 251 insertions(+), 1 deletion(-)
diff --git a/configure.ac b/configure.ac
index af3fe28..8668e60 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1418,7 +1418,6 @@ platform])
fi
AM_CONDITIONAL([VIR_CHRDEV_LOCK_FILE_PATH], [test "$with_chrdev_lock_files" !=
"no"])
-
AC_ARG_WITH([secdriver-selinux],
[AS_HELP_STRING([--with-secdriver-selinux],
[use SELinux security driver @<:@default=check@:>@])],
diff --git a/libvirt.spec.in b/libvirt.spec.in
index f491de7..762b404 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1948,6 +1948,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
@@ -2050,6 +2051,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
diff --git a/src/Makefile.am b/src/Makefile.am
index f69923f..78ef54e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2643,6 +2643,7 @@ endif WITH_SANLOCK
if WITH_QEMU
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/channel/target"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/nvram"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt/qemu"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/log/libvirt/qemu"
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index e7db7fe..62951da 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -88,6 +88,8 @@ module Libvirtd_qemu =
let log_entry = bool_entry "log_timestamp"
+ let nvram_entry = str_array_entry "nvram"
+
(* Each entry in the config is one of the following ... *)
let entry = vnc_entry
| spice_entry
@@ -100,6 +102,7 @@ module Libvirtd_qemu =
| rpc_entry
| network_entry
| log_entry
+ | nvram_entry
let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^
\t\n][^\n]*)?/ . del /\n/ "\n" ]
let empty = [ label "#empty" . eol ]
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.
On 15.08.2014 16:13, Laszlo Ersek wrote:
On 08/15/14 15:43, Michal Privoznik wrote:
Up to now, users can configure BIOS via the 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: . Currently, it has no additional
attributes, it's just a bare element containing path to the variable
store file.
I compared this version against v1 1/3, and my earlier notes on that v1
patch. It looks good to me.
I don't know enough about libvirt to give an R-b that's really worth its
face value, so I'll just ack.
Acked-by: Laszlo Ersek
In addition, is there an easy way for me to test this patchset? I can
pluck the series from the list, apply it manually to my upstream clone,
build etc. My main question is if there's going to be some interference
with my "normal", RHEL-7, system-wide libvirtd installation.
No, there shouldn't be any interference unless you 'make install'.
If I follow <http://libvirt.org/deployment.html> and just install (as
non-root) to a private --prefix, will that just work? I vaguely remember
that I did get this working once before (when I was working on commits
ccca5dc3 and 51e184e9), but I don't remember any longer.
You don't need even need to use that. I have libvirt installed via the
packaging system on my distribution too and all I do is:
./autogen.sh --system && make
then I run (as root) ./run daemon/libvirtd and that's it.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
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
---
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(+)
diff --git a/libvirt.spec.in b/libvirt.spec.in
index f491de7..762b404 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -1948,6 +1948,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
@@ -2050,6 +2051,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
diff --git a/src/Makefile.am b/src/Makefile.am
index f69923f..78ef54e 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2643,6 +2643,7 @@ endif WITH_SANLOCK
if WITH_QEMU
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/channel/target"
+ $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu/nvram"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt/qemu"
$(MKDIR_P) "$(DESTDIR)$(localstatedir)/log/libvirt/qemu"
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index e7db7fe..62951da 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -88,6 +88,8 @@ module Libvirtd_qemu =
let log_entry = bool_entry "log_timestamp"
+ let nvram_entry = str_array_entry "nvram"
+
(* Each entry in the config is one of the following ... *)
let entry = vnc_entry
| spice_entry
@@ -100,6 +102,7 @@ module Libvirtd_qemu =
| rpc_entry
| network_entry
| log_entry
+ | nvram_entry
let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^
\t\n][^\n]*)?/ . del /\n/ "\n" ]
let empty = [ label "#empty" . eol ]
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" ]
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 238d2b1..195fe8e 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -107,6 +107,9 @@ void qemuDomainCmdlineDefFree(qemuDomainCmdlineDefPtr def)
VIR_FREE(def);
}
+#define VIR_QEMU_LOADER_FILE_PATH "/usr/share/OVMF/OVMF_CODE.fd"
+#define VIR_QEMU_NVRAM_FILE_PATH "/usr/share/OVMF/OVMF_VARS.fd"
+
virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged)
{
vi
On 18.08.2014 17:28, Eric Blake wrote:
On 08/14/2014 02:24 AM, Michal Privoznik wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1103245
An advice appeared there on the qemu-devel list [1]. When a domain is
suspended and then resumed guest kernel is not aware of this. So we've
introduced virDomainSetTime API that resets the time within guest
using qemu-ga. On the other hand, qemu itself is trying to make RTC
beat faster to catch the difference. But if we don't tell qemu that
guest's time was reset via the other method, both mechanisms are
applied resulting in again wrong guest time. In order to avoid summing
both corrections we need to tell qemu that it should not use the RTC
injection if the guest time is set via guest agent.
1: http://www.mail-archive.com/qemu-devel@nongnu.org/msg236435.html
Signed-off-by: Michal Privoznik
---
Notes:
diff to v1:
-fixed command name in subject
-added testcase
+++ b/src/qemu/qemu_driver.c
@@ -16879,6 +16879,16 @@ qemuDomainSetTime(virDomainPtr dom,
rv = qemuAgentSetTime(priv->agent, seconds, nseconds, rtcSync);
qemuDomainObjExitAgent(vm);
+if (!virDomainObjIsActive(vm)) {
+virReportError(VIR_ERR_OPERATION_INVALID,
+ "%s", _("domain is not running"));
+goto endjob;
+}
+
+qemuDomainObjEnterMonitor(driver, vm);
+rv = qemuMonitorRTCResetReinjection(priv->mon);
+qemuDomainObjExitMonitor(driver, vm);
We have four combinations:
1. old qemu, old qga: command fails because qga doesn't support it, qemu
tries to catch up time manually (might eventually match real time)
2. new qemu, old qga: command fails because qga doesn't support it, qemu
tries to catch up time manually (might eventually match real time)
3. new qemu, new qga: both qga and qemu commands work, no additional
catchup attempted and guest is now accurate
4. old qemu, new qga: qga succeeds, but qemu command fails, so we have
overcorrected and qemu is trying to catch up time manually
(overcorrected, so it cannot match real time)
I guess reporting failure in those three cases is fine, although I'm
still worried about case 4. I'd feel a lot better if there were a
qemu_capabilities.h bit that detects if the qemu command is present, and
skip even attempting the qga command unless we ALSO know the qemu
command is present (that is, use the capability check to completely
avoid case 4, by turning it into the same behavior as case 1).
Okay. Although I've just realized one (corner) case. From my
understanding of rtc-reset-reinjection time it's only necessary if guest
was suspended for a while and the guest's RTC clock skewed.
But what if I start fresh new guest and just want to set its time (leave
aside the reasoning why would I do that for a while)? Is the
rtc-reset-reinjection necessary? I wouldn't say. But on the other hand -
libvirt doesn't know if the RTC is synced already or not. Hence it's
safer for libvirt to issue the command every single time.
In fact, there are two ways to set guest time:
a) {"execute":"guest-set-time"}
b) {"execute":"guest-set-time, "arguments":{"time":1234567890}}
While in the case a) guest time is set by reading from guest's RTC, in
case of b) guest time is set by calling settimeofday() and RTC is
written thereafter.
So is the rtc-reset-reinjection necessary only for case a) and in case
b) QEMU somehow detects RTC write and cancels the reinjection itself?
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
