[libvirt] [PATCH] virsh: Fix help info of net-event and event command
From: Li Yang liyang.f...@cn.fujitsu.com
For now the help informatin of net-event and event like this:
[root@localhost qemu]# virsh help net-event
NAME
net-event - (null)
...
[root@localhost qemu]# virsh help event
NAME
event - (null)
...
Now fixed them, make them show correct information
instead of 'null'.
Signed-off-by: Li Yang liyang.f...@cn.fujitsu.com
---
tools/virsh-domain.c |2 +-
tools/virsh-network.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index f2856a5..310b5dc 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -11030,7 +11030,7 @@ static vshEventCallback vshEventCallbacks[] = {
verify(VIR_DOMAIN_EVENT_ID_LAST == ARRAY_CARDINALITY(vshEventCallbacks));
static const vshCmdInfo info_event[] = {
-{.name = event,
+{.name = help,
.data = N_(Domain Events)
},
{.name = desc,
diff --git a/tools/virsh-network.c b/tools/virsh-network.c
index 22d21e7..338db28 100644
--- a/tools/virsh-network.c
+++ b/tools/virsh-network.c
@@ -1180,7 +1180,7 @@ vshEventLifecyclePrint(virConnectPtr conn
ATTRIBUTE_UNUSED,
}
static const vshCmdInfo info_network_event[] = {
-{.name = net-event,
+{.name = help,
.data = N_(Network Events)
},
{.name = desc,
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Define STATE_XMLFILE_LEN as 8192
On 04/03/2014 08:43 AM, Olivia Yin wrote:
Save/restore the state of domain and migrate need read state XML file.
8192B is not the exactly maximum file length of state file.
restore command could work successfully in virsh shell.
virsh # list --all
IdName State
9 sdkrunning
virsh # save sdk pp
Domain sdk saved to pp
virsh # list --all
IdName State
- sdkshut off
virsh # restore pp
Domain restored from pp
virsh # list --all
IdName State
10sdkrunning
But it will fail with 'virsh restore' command because the state file is
oversized.
~# virsh restore pp
error: Failed to read file 'pp': Value too large for defined data type
There is no xml file supplied here, just the save file...
Signed-off-by: Olivia Yin hong-hua@freescale.com
---
tools/virsh-domain.c | 6 +++---
tools/virsh.h| 1 +
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 73414f8..8ade296 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3547,7 +3547,7 @@ doSave(void *opaque)
goto out;
if (xmlfile
-virFileReadAll(xmlfile, 8192, xml) 0) {
+virFileReadAll(xmlfile, STATE_XMLFILE_LEN, xml) 0) {
vshReportError(ctl);
goto out;
}
@@ -3840,7 +3840,7 @@ cmdSaveImageDefine(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, xml, xmlfile) 0)
return false;
-if (virFileReadAll(xmlfile, 8192, xml) 0)
+if (virFileReadAll(xmlfile, STATE_XMLFILE_LEN, xml) 0)
goto cleanup;
if (virDomainSaveImageDefineXML(ctl-conn, file, xml, flags) 0) {
@@ -4424,7 +4424,7 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd)
return false;
if (xmlfile
-virFileReadAll(xmlfile, 8192, xml) 0)
+virFileReadAll(xmlfile, STATE_XMLFILE_LEN, xml) 0)
... but this changes the limit for the XML file of the updated domain, so it
shouldn't be executed by the above command. Does this actually fix the issue?
Also, VSH_MAX_XML_FILE can be used for all of these.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 2/3] qemu: RDMA migration support using 'rdma' URI
On 02/03/2014 11:19 PM, Jiri Denemark wrote:
USAGE: $ virsh migrate --live --migrateuri x-rdma:hostname domain
qemu+ssh://hostname/system
s/x-rdma/rdma/ and I believe we should use rdma://hostname as the URI
Acknowledged.
Signed-off-by: Michael R. Hines mrhi...@us.ibm.com
---
src/qemu/qemu_capabilities.c | 13 +
src/qemu/qemu_capabilities.h | 1 +
src/qemu/qemu_command.c | 8
src/qemu/qemu_migration.c| 110 ++-
src/qemu/qemu_monitor.c | 3 +-
src/qemu/qemu_monitor.h | 1 +
src/util/viruri.c| 7 ++-
7 files changed, 119 insertions(+), 24 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index 548b988..d82b48c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -243,6 +243,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
virtio-mmio,
ich9-intel-hda,
kvm-pit-lost-tick-policy,
+
+ migrate-qemu-rdma, /* 160 */
s/migrate-qemu-rdma/migrate-rdma/
qemu string in pretty redundant here given that it is a qemu
capability.
Acknowledged.
);
struct _virQEMUCaps {
@@ -906,6 +908,9 @@ virCapsPtr virQEMUCapsInit(virQEMUCapsCachePtr cache)
virCapabilitiesAddHostMigrateTransport(caps,
tcp);
+virCapabilitiesAddHostMigrateTransport(caps,
+ rdma);
+
/* QEMU can support pretty much every arch that exists,
* so just probe for them all - we gracefully fail
* if a qemu-system-$ARCH binary can't be found
@@ -1110,6 +1115,7 @@ virQEMUCapsComputeCmdFlags(const char *help,
* -incoming unix (qemu = 0.12.0)
* -incoming fd (qemu = 0.12.0)
* -incoming stdio (all earlier kvm)
+ * -incoming rdma (qemu = 2.0.0)
*
* NB, there was a pre-kvm-79 'tcp' support, but it
* was broken, because it blocked the monitor console
@@ -1130,6 +1136,9 @@ virQEMUCapsComputeCmdFlags(const char *help,
virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_KVM_STDIO);
}
+if (version = 200)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_RDMA);
+
if (version = 1)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_0_10);
This is not needed, we won't be parsing -help for any QEMU that supports
RDMA.
Acknowledged.
@@ -2561,6 +2570,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps,
if (qemuCaps-version = 1006000)
virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY);
+if (qemuCaps-version = 200)
+virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_RDMA);
+
+
And here we need a better check for rdma migration. What if someone
compiles QEMU without RDMA support?
if (virQEMUCapsProbeQMPCommands(qemuCaps, mon) 0)
goto cleanup;
if (virQEMUCapsProbeQMPEvents(qemuCaps, mon) 0)
diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
index 02d47c6..3e78961 100644
--- a/src/qemu/qemu_capabilities.h
+++ b/src/qemu/qemu_capabilities.h
@@ -198,6 +198,7 @@ enum virQEMUCapsFlags {
QEMU_CAPS_DEVICE_VIRTIO_MMIO = 157, /* -device virtio-mmio */
QEMU_CAPS_DEVICE_ICH9_INTEL_HDA = 158, /* -device ich9-intel-hda */
QEMU_CAPS_KVM_PIT_TICK_POLICY = 159, /* kvm-pit.lost_tick_policy */
+QEMU_CAPS_MIGRATE_QEMU_RDMA = 160, /* have qemu rdma migration */
s/_QEMU// as it is redundant.
Acknowledged.
QEMU_CAPS_LAST, /* this must always be the last item */
};
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 763417f..0d23d8b 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9448,6 +9448,14 @@ qemuBuildCommandLine(virConnectPtr conn,
goto error;
}
virCommandAddArg(cmd, migrateFrom);
+} else if (STRPREFIX(migrateFrom, rdma)) {
+if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_RDMA)) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ %s, _(RDMA migration is not supported with
+ this QEMU binary));
+goto error;
+}
+virCommandAddArg(cmd, migrateFrom);
} else if (STREQ(migrateFrom, stdio)) {
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_FD)) {
virCommandAddArgFormat(cmd, fd:%d, migrateFd);
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index ef6f1c5..1e0f538 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -46,6 +46,7 @@
#include virerror.h
#include viralloc.h
#include virfile.h
+#include virprocess.h
#include datatypes.h
#include fdstream.h
#include viruuid.h
@@ -2163,6 +2164,7 @@ qemuMigrationPrepareAny(virQEMUDriverPtr driver,
Re: [libvirt] [PATCH v2] Define CPUINFO_FILE_LEN and fix maxlen of cpuinfo file for all uses
On 04/04/2014 03:21 AM, Olivia Yin wrote:
For example, the file /proc/cpuinfo for 24 cores PowerPC platform is larger
than
the previous maximum size 2KB.
It will fail to start libvirtd with the error message as below:
virFileReadAll: Failed to read file '/proc/cpuinfo': Value too large for
defined
data type
virSysinfoRead: internal error Failed to open /proc/cpuinfo
This patch defines CPUINFO_FILE_LEN as 10KB which is enough for most
architectures.
Signed-off-by: Olivia Yin hong-hua@freescale.com
---
src/util/virsysinfo.c | 6 +++---
src/util/virsysinfo.h | 2 ++
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index 7b16157..873872c 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -223,7 +223,7 @@ virSysinfoRead(void)
if (VIR_ALLOC(ret) 0)
goto no_memory;
-if (virFileReadAll(CPUINFO, 2048, outbuf) 0) {
+if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, outbuf) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to open %s), CPUINFO);
return NULL;
@@ -341,7 +341,7 @@ virSysinfoRead(void)
if (VIR_ALLOC(ret) 0)
goto no_memory;
-if (virFileReadAll(CPUINFO, 2048, outbuf) 0) {
+if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, outbuf) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to open %s), CPUINFO);
return NULL;
@@ -470,7 +470,7 @@ virSysinfoRead(void)
goto no_memory;
/* Gather info from /proc/cpuinfo */
-if (virFileReadAll(CPUINFO, 8192, outbuf) 0) {
+if (virFileReadAll(CPUINFO, CPUINFO_FILE_LEN, outbuf) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_(Failed to open %s), CPUINFO);
return NULL;
diff --git a/src/util/virsysinfo.h b/src/util/virsysinfo.h
index fbb505b..b5336e2 100644
--- a/src/util/virsysinfo.h
+++ b/src/util/virsysinfo.h
@@ -102,4 +102,6 @@ bool virSysinfoIsEqual(virSysinfoDefPtr src,
VIR_ENUM_DECL(virSysinfo)
+#define CPUINFO_FILE_LEN (10*1024) /*10KB limit for /proc/cpuinfo file*/
+
This doesn't need to be in the header file.
I moved it near the definition of CPUINFO and pushed it. Thank you for the
patch!
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix Memory Leak in daemon/libvirtd.c
On 04/03/2014 08:13 PM, Nehal J Wani wrote:
Fixes leak introduced by e562e82f
==4937== 64 bytes in 1 blocks are definitely lost in loss record 270 of 405
==4937==at 0x4A06BE0: realloc (vg_replace_malloc.c:662)
==4937==by 0x6FA41C4: __vasprintf_chk (vasprintf_chk.c:90)
==4937==by 0x50C8D29: virVasprintfInternal (stdio2.h:199)
==4937==by 0x50C8E3A: virAsprintfInternal (virstring.c:362)
==4937==by 0x11D01A: main (libvirtd.c:1170)
---
daemon/libvirtd.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index e247259..bb84c90 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -1132,6 +1132,7 @@ int main(int argc, char **argv) {
bool privileged = geteuid() == 0 ? true : false;
bool implicit_conf = false;
char *run_dir = NULL;
+char *cpumap = NULL;
mode_t old_umask;
struct option opts[] = {
@@ -1159,7 +1160,6 @@ int main(int argc, char **argv) {
if (strstr(argv[0], lt-libvirtd) ||
strstr(argv[0], /daemon/.libs/libvirtd)) {
char *tmp = strrchr(argv[0], '/');
-char *cpumap;
There is no need to move the declaration or initialize it to NULL, since it's
always declared and initialized when we get to the VIR_FREE below and we don't
have cleanup paths here.
if (!tmp) {
fprintf(stderr, _(%s: cannot identify driver directory\n),
argv[0]);
exit(EXIT_FAILURE);
@@ -1182,6 +1182,7 @@ int main(int argc, char **argv) {
virDriverModuleInitialize(driverdir);
#endif
cpuMapOverride(cpumap);
+VIR_FREE(cpumap);
*tmp = '/';
/* Must not free 'driverdir' - it is still used */
}
ACK and pushed, thank you for the patch!
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Fix help info of net-event and event command
On 04/04/2014 07:58 AM, liyang wrote: From: Li Yang liyang.f...@cn.fujitsu.com For now the help informatin of net-event and event like this: [root@localhost qemu]# virsh help net-event NAME net-event - (null) ... [root@localhost qemu]# virsh help event NAME event - (null) ... Now fixed them, make them show correct information instead of 'null'. Signed-off-by: Li Yang liyang.f...@cn.fujitsu.com --- tools/virsh-domain.c |2 +- tools/virsh-network.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) This has already been fixed: commit 14d7fcc23a47e9e8911e8bba0adcd8cf51727779 Author: Eric Blake ebl...@redhat.com CommitDate: 2014-03-31 08:37:39 -0600 virsh: fix 'help event' Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 2/3] qemu: RDMA migration support using 'rdma' URI
On 02/03/2014 11:44 PM, Eric Blake wrote: On 02/03/2014 08:19 AM, Jiri Denemark wrote: On Mon, Jan 13, 2014 at 14:28:11 +0800, mrhi...@linux.vnet.ibm.com wrote: From: Michael R. Hines mrhi...@us.ibm.com The switch from x-rdma = rdma has not yet happened, but at least we can review the patch until it goes through on qemu-devel. The paragraph above would better fit after --- below so that it disappears once this patch gets applied as the statement won't be valid anymore at that time. USAGE: $ virsh migrate --live --migrateuri x-rdma:hostname domain qemu+ssh://hostname/system s/x-rdma/rdma/ and I believe we should use rdma://hostname as the URI Full documenation of the feature: http://wiki.qemu.org/Features/RDMALiveMigration s/documenation/documentation/ @@ -2561,6 +2570,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps-version = 1006000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); +if (qemuCaps-version = 200) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_RDMA); + + And here we need a better check for rdma migration. What if someone compiles QEMU without RDMA support? Better than hard-coding it to a version string is to probe the results of query-migrate-capabilities and only setting the capability if the resulting list includes rdma-pin-all, as that will serve as a reliable witness of qemu being new enough to support rdma without an x- prefix. These comments I don't understand: Why can't we depend on the version number here? Isn't that what it was designed for? If someone compiles QEMU without RDMA support - why does libvirt need to know about that? Shouldn't the admin know what their hardware is capable of - otherwise, if they try to specify rdma://hostname as a migration option, they will get a failure - which would be the correct behavior - they tried to do something without verifying that their hardware was capable of handling it. Checking the capability list won't help here either: It will still be in the list even if we don't compile QEMU with RDMA support. And if someone sets the capability anyway, it will just get ignored by QEMU since RDMA support was not available at compile time. - Michael -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 3/3] qemu: memory pre-pinning support for RDMA migration
On 02/04/2014 10:56 PM, Jiri Denemark wrote:
On Mon, Jan 13, 2014 at 14:28:12 +0800, mrhi...@linux.vnet.ibm.com wrote:
From: Michael R. Hines mrhi...@us.ibm.com
RDMA Live migration requires registering memory with the hardware,
Hmm, I forgot to ask when I was reviewing the previous patch but does
any of this RDMA migration functionality require special privileges or
is any unprivileged process able to use RDMA?
No, it does not require any special privileges (just like HPC programs),
but it does access a bunch of special device files (unprivleged):
I believe someone recommended that we had the list of those
device files to the default list of allowed devices that libvirt is
already maintaining.
I'll include them in the next patch
+{
+qemuDomainObjPrivatePtr priv = vm-privateData;
+int ret;
+
+if (qemuDomainObjEnterMonitorAsync(driver, vm, job) 0)
+return -1;
+
+ret = qemuMonitorGetMigrationCapability(
+priv-mon,
+QEMU_MONITOR_MIGRATION_CAPS_RDMA_PIN_ALL);
Is this capability always present when QEMU supports RDMA migration? If
so, it could be used when we check if QEMU supports RDMA migration.
See my comment earlier in the thread...
Yes, it's present, but it still does not guarantee that QEMU supports
it if RDMA was compiled out - only the version number is a
(minimal) guarantee, and even then the hardware can still throw
an error if RDMA itself is not supported.
I'm still not seeing what's wrong with depending on the version
number since other features are also depending on the version
number.
Are you guys suggesting that we stop depending on version altogether
for *all* QEMU features?
+unsigned long long memKB = vm-def-mem.hard_limit ?
+ vm-def-mem.hard_limit :
+vm-def-mem.max_balloon + 1024 * 1024;
+virProcessSetMaxMemLock(vm-pid, memKB * 3);
Apart from weird indentation of the virProcessSetMaxMemLock line, there
are several issues here:
- this code should be moved inside qemuMigrationSetPinAll and done only
if VIR_MIGRATE_RDMA_PIN_ALL flag was used.
- virProcessSetMaxMemLock wants the value in bytes, thus memKB should
rather by multiplied by 1024.
- what memory is going to be mlock()ed with rdma-pin-all? Is it going to
be all memory allocated by QEMU or just the domain's memory? If it's
all QEMU memory, we already painfully found out it's impossible to
automatically compute how much memory QEMU consumes in addition to the
configured domain's memory and I think a better approach would be to
just migration with rdma-pin-all unless hard_limit is specified.
(Acknowledged for the first two comments).
Regarding your 3rd part: That's why I multiplied the number by 3,
the RDMA code *must* lock or the whole thing falls apart, so
we have to make some kind of reasonable assumption on how
much to set the lock limit to.
I'm willing to go even higher than 3 times the limit, if nobody objects,
but we have to pick some kind of limit..somehow.
Comments?
- Michael
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: Fix comment of vshCmdInfo
From: Li Yang liyang.f...@cn.fujitsu.com
The original comment of vshCmdInfo:
name - command name
Actually it's 'help' and the short description
of command, not the command name.
Signed-off-by: Li Yang liyang.f...@cn.fujitsu.com
---
tools/virsh.h |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/tools/virsh.h b/tools/virsh.h
index 3e0251b..1ffc003 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -154,7 +154,7 @@ typedef char **(*vshCompleter)(unsigned int flags);
* vshCmdInfo -- name/value pair for information about command
*
* Commands should have at least the following names:
- * name - command name
+ * help - short description of command
* desc - description of command, or empty string
*/
struct _vshCmdInfo {
--
1.7.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Define STATE_XMLFILE_LEN as 8192
This patch only defines a macro STATE_FILE_LEN.
It doesn't aim to fix the issue.
VSH_MAX_XML_FILE is 10MB, but I got a saved XML file about 180MB.
Best Regards,
Olivia
-Original Message-
From: Ján Tomko [mailto:jto...@redhat.com]
Sent: Friday, April 04, 2014 2:06 PM
To: Yin Olivia-R63875; libvir-list@redhat.com
Subject: Re: [libvirt] [PATCH] Define STATE_XMLFILE_LEN as 8192
On 04/03/2014 08:43 AM, Olivia Yin wrote:
Save/restore the state of domain and migrate need read state XML file.
8192B is not the exactly maximum file length of state file.
restore command could work successfully in virsh shell.
virsh # list --all
IdName State
9 sdkrunning
virsh # save sdk pp
Domain sdk saved to pp
virsh # list --all
IdName State
- sdkshut off
virsh # restore pp
Domain restored from pp
virsh # list --all
IdName State
10sdkrunning
But it will fail with 'virsh restore' command because the state file is
oversized.
~# virsh restore pp
error: Failed to read file 'pp': Value too large for defined data type
There is no xml file supplied here, just the save file...
Signed-off-by: Olivia Yin hong-hua@freescale.com
---
tools/virsh-domain.c | 6 +++---
tools/virsh.h| 1 +
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index
73414f8..8ade296 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -3547,7 +3547,7 @@ doSave(void *opaque)
goto out;
if (xmlfile
-virFileReadAll(xmlfile, 8192, xml) 0) {
+virFileReadAll(xmlfile, STATE_XMLFILE_LEN, xml) 0) {
vshReportError(ctl);
goto out;
}
@@ -3840,7 +3840,7 @@ cmdSaveImageDefine(vshControl *ctl, const vshCmd
*cmd)
if (vshCommandOptStringReq(ctl, cmd, xml, xmlfile) 0)
return false;
-if (virFileReadAll(xmlfile, 8192, xml) 0)
+if (virFileReadAll(xmlfile, STATE_XMLFILE_LEN, xml) 0)
goto cleanup;
if (virDomainSaveImageDefineXML(ctl-conn, file, xml, flags) 0)
{ @@ -4424,7 +4424,7 @@ cmdRestore(vshControl *ctl, const vshCmd *cmd)
return false;
if (xmlfile
-virFileReadAll(xmlfile, 8192, xml) 0)
+virFileReadAll(xmlfile, STATE_XMLFILE_LEN, xml) 0)
... but this changes the limit for the XML file of the updated domain, so
it shouldn't be executed by the above command. Does this actually fix the
issue?
Also, VSH_MAX_XML_FILE can be used for all of these.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5] tests: use C99 initialization for storage test
On 04/04/14 06:32, Eric Blake wrote: Writing this test with C99 initializers will make it easier to test additions and deletions to struct members as I refactor the code. * tests/virstoragetest.c (mymain): Rewrite initializers. Signed-off-by: Eric Blake ebl...@redhat.com --- tests/virstoragetest.c | 105 + 1 file changed, 80 insertions(+), 25 deletions(-) ACK, Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/6] qemuDomainGetPercpuStats cleanups
This series removes qemuDomainGetPercpuStats in favor of virCgroupGetPercpuStats, fixes incorrect startcpu boundaries check and cleans the code up. Ján Tomko (6): Don't require domain obj in qemuDomainGetPercpuStats Fix return value of virCgroupGetPercpuStats Extend virCgroupGetPercpuStats to fill in vcputime too Rename id, max_id to need_cpus, total_cpus Check maximum startcpu value correctly Clean up virCgroupGetPercpuStats src/lxc/lxc_driver.c | 2 +- src/qemu/qemu_driver.c | 163 + src/util/vircgroup.c | 115 -- src/util/vircgroup.h | 3 +- tests/vircgrouptest.c | 2 +- 5 files changed, 103 insertions(+), 182 deletions(-) -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/6] Fix return value of virCgroupGetPercpuStats
We need to return the number of successfully populated stats, not the nparams supplied by the user. --- src/util/vircgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index b881c8c..1ff3dad 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2897,7 +2897,7 @@ virCgroupGetPercpuStats(virCgroupPtr group, goto cleanup; } -rv = nparams; +rv = param_idx + 1; cleanup: VIR_FREE(buf); -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 6/6] Clean up virCgroupGetPercpuStats
The iterator is checked for being less than or equal to need_cpus.
The 'n' variable is incremented need_cpus + 1 times.
Simplify the computation of need_cpus and make its value one larger,
to let it be used instead of 'n' and compared without the equal sign
in loop conditions.
Just index the sum_cpu_time array instead of using a helper variable.
Start the loop at start_cpu instead of continuing for all lower values.
---
src/util/vircgroup.c | 32 +---
1 file changed, 9 insertions(+), 23 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 2272bc6..74e0907 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -2904,8 +2904,6 @@ virCgroupGetPercpuStats(virCgroupPtr group,
char *pos;
char *buf = NULL;
unsigned long long *sum_cpu_time = NULL;
-unsigned long long *sum_cpu_pos;
-unsigned int n = 0;
virTypedParameterPtr ent;
int param_idx;
unsigned long long cpu_time;
@@ -2919,14 +2917,11 @@ virCgroupGetPercpuStats(virCgroupPtr group,
}
/* To parse account file, we need to know how many cpus are present. */
-total_cpus = nodeGetCPUCount();
-if (total_cpus 0)
+if ((total_cpus = nodeGetCPUCount()) 0)
return rv;
-if (ncpus == 0) {
-rv = total_cpus;
-goto cleanup;
-}
+if (ncpus == 0)
+return total_cpus;
if (start_cpu = total_cpus) {
virReportError(VIR_ERR_INVALID_ARG,
@@ -2944,18 +2939,13 @@ virCgroupGetPercpuStats(virCgroupPtr group,
param_idx = 0;
/* number of cpus to compute */
-if (start_cpu = total_cpus - ncpus)
-need_cpus = total_cpus - 1;
-else
-need_cpus = start_cpu + ncpus - 1;
+need_cpus = MIN(total_cpus, start_cpu + ncpus);
-for (i = 0; i = need_cpus; i++) {
+for (i = 0; i need_cpus; i++) {
if (virStrToLong_ull(pos, pos, 10, cpu_time) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(cpuacct parse error));
goto cleanup;
-} else {
-n++;
}
if (i start_cpu)
continue;
@@ -2970,21 +2960,17 @@ virCgroupGetPercpuStats(virCgroupPtr group,
/* return percpu vcputime in index 1 */
param_idx++;
-if (VIR_ALLOC_N(sum_cpu_time, n) 0)
+if (VIR_ALLOC_N(sum_cpu_time, need_cpus) 0)
goto cleanup;
-if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, n) 0)
+if (virCgroupGetPercpuVcpuSum(group, nvcpupids, sum_cpu_time, need_cpus)
0)
goto cleanup;
-sum_cpu_pos = sum_cpu_time;
-for (i = 0; i = need_cpus; i++) {
-cpu_time = *(sum_cpu_pos++);
-if (i start_cpu)
-continue;
+for (i = start_cpu; i need_cpus; i++) {
if (virTypedParameterAssign(params[(i - start_cpu) * nparams +
param_idx],
VIR_DOMAIN_CPU_STATS_VCPUTIME,
VIR_TYPED_PARAM_ULLONG,
-cpu_time) 0)
+sum_cpu_time[i]) 0)
goto cleanup;
}
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/6] Extend virCgroupGetPercpuStats to fill in vcputime too
Currently, virCgroupGetPercpuStats is only used by the LXC driver,
filling out the CPUTIME stats. qemuDomainGetPercpuStats does this
and also filles out VCPUTIME stats.
Extend virCgroupGetPercpuStats to also report VCPUTIME stats if
nvcpupids is non-zero. In the LXC driver, we don't have cpupids.
In the QEMU driver, there is at least one cpupid for a running domain,
so the behavior shouldn't change for QEMU either.
Also rename getSumVcpuPercpuStats to virCgroupGetPercpuVcpuSum.
---
src/lxc/lxc_driver.c | 2 +-
src/qemu/qemu_driver.c | 163 +
src/util/vircgroup.c | 99 +-
src/util/vircgroup.h | 3 +-
tests/vircgrouptest.c | 2 +-
5 files changed, 102 insertions(+), 167 deletions(-)
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index b900bc6..60f741f 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -5659,7 +5659,7 @@ lxcDomainGetCPUStats(virDomainPtr dom,
params, nparams);
else
ret = virCgroupGetPercpuStats(priv-cgroup, params,
- nparams, start_cpu, ncpus);
+ nparams, start_cpu, ncpus, 0);
cleanup:
if (vm)
virObjectUnlock(vm);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index da976b3..68e2741 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15977,165 +15977,6 @@ qemuDomainGetMetadata(virDomainPtr dom,
return ret;
}
-/* This function gets the sums of cpu time consumed by all vcpus.
- * For example, if there are 4 physical cpus, and 2 vcpus in a domain,
- * then for each vcpu, the cpuacct.usage_percpu looks like this:
- * t0 t1 t2 t3
- * and we have 2 groups of such data:
- * v\p 0 1 2 3
- * 0 t00 t01 t02 t03
- * 1 t10 t11 t12 t13
- * for each pcpu, the sum is cpu time consumed by all vcpus.
- * s0 = t00 + t10
- * s1 = t01 + t11
- * s2 = t02 + t12
- * s3 = t03 + t13
- */
-static int
-getSumVcpuPercpuStats(virCgroupPtr group,
- unsigned int nvcpupids,
- unsigned long long *sum_cpu_time,
- unsigned int num)
-{
-int ret = -1;
-size_t i;
-char *buf = NULL;
-virCgroupPtr group_vcpu = NULL;
-
-for (i = 0; i nvcpupids; i++) {
-char *pos;
-unsigned long long tmp;
-size_t j;
-
-if (virCgroupNewVcpu(group, i, false, group_vcpu) 0)
-goto cleanup;
-
-if (virCgroupGetCpuacctPercpuUsage(group_vcpu, buf) 0)
-goto cleanup;
-
-pos = buf;
-for (j = 0; j num; j++) {
-if (virStrToLong_ull(pos, pos, 10, tmp) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(cpuacct parse error));
-goto cleanup;
-}
-sum_cpu_time[j] += tmp;
-}
-
-virCgroupFree(group_vcpu);
-VIR_FREE(buf);
-}
-
-ret = 0;
- cleanup:
-virCgroupFree(group_vcpu);
-VIR_FREE(buf);
-return ret;
-}
-
-static int
-qemuDomainGetPercpuStats(virCgroupPtr group,
- virTypedParameterPtr params,
- unsigned int nparams,
- int start_cpu,
- unsigned int ncpus,
- unsigned int nvcpupids)
-{
-int rv = -1;
-size_t i;
-int id, max_id;
-char *pos;
-char *buf = NULL;
-unsigned long long *sum_cpu_time = NULL;
-unsigned long long *sum_cpu_pos;
-unsigned int n = 0;
-virTypedParameterPtr ent;
-int param_idx;
-unsigned long long cpu_time;
-
-/* return the number of supported params */
-if (nparams == 0 ncpus != 0)
-return QEMU_NB_PER_CPU_STAT_PARAM;
-
-/* To parse account file, we need to know how many cpus are present. */
-max_id = nodeGetCPUCount();
-if (max_id 0)
-return rv;
-
-if (ncpus == 0) { /* returns max cpu ID */
-rv = max_id;
-goto cleanup;
-}
-
-if (start_cpu max_id) {
-virReportError(VIR_ERR_INVALID_ARG,
- _(start_cpu %d larger than maximum of %d),
- start_cpu, max_id);
-goto cleanup;
-}
-
-/* we get percpu cputime accounting info. */
-if (virCgroupGetCpuacctPercpuUsage(group, buf))
-goto cleanup;
-pos = buf;
-
-/* return percpu cputime in index 0 */
-param_idx = 0;
-
-/* number of cpus to compute */
-if (start_cpu = max_id - ncpus)
-id = max_id - 1;
-else
-id = start_cpu + ncpus - 1;
-
-for (i = 0; i = id; i++) {
-if (virStrToLong_ull(pos, pos, 10, cpu_time) 0) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(cpuacct parse error));
-goto cleanup;
-} else {
-n++;
-}
-if (i start_cpu)
-
[libvirt] [PATCH 1/6] Don't require domain obj in qemuDomainGetPercpuStats
All we need is the virCgroupPtr and number of vcpupids.
This will allow the function to be moved to util/vircgroup.c.
---
src/qemu/qemu_driver.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1d08951..da976b3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15992,22 +15992,22 @@ qemuDomainGetMetadata(virDomainPtr dom,
* s3 = t03 + t13
*/
static int
-getSumVcpuPercpuStats(virDomainObjPtr vm,
+getSumVcpuPercpuStats(virCgroupPtr group,
+ unsigned int nvcpupids,
unsigned long long *sum_cpu_time,
unsigned int num)
{
int ret = -1;
size_t i;
char *buf = NULL;
-qemuDomainObjPrivatePtr priv = vm-privateData;
virCgroupPtr group_vcpu = NULL;
-for (i = 0; i priv-nvcpupids; i++) {
+for (i = 0; i nvcpupids; i++) {
char *pos;
unsigned long long tmp;
size_t j;
-if (virCgroupNewVcpu(priv-cgroup, i, false, group_vcpu) 0)
+if (virCgroupNewVcpu(group, i, false, group_vcpu) 0)
goto cleanup;
if (virCgroupGetCpuacctPercpuUsage(group_vcpu, buf) 0)
@@ -16035,11 +16035,12 @@ getSumVcpuPercpuStats(virDomainObjPtr vm,
}
static int
-qemuDomainGetPercpuStats(virDomainObjPtr vm,
+qemuDomainGetPercpuStats(virCgroupPtr group,
virTypedParameterPtr params,
unsigned int nparams,
int start_cpu,
- unsigned int ncpus)
+ unsigned int ncpus,
+ unsigned int nvcpupids)
{
int rv = -1;
size_t i;
@@ -16049,7 +16050,6 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm,
unsigned long long *sum_cpu_time = NULL;
unsigned long long *sum_cpu_pos;
unsigned int n = 0;
-qemuDomainObjPrivatePtr priv = vm-privateData;
virTypedParameterPtr ent;
int param_idx;
unsigned long long cpu_time;
@@ -16076,7 +16076,7 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm,
}
/* we get percpu cputime accounting info. */
-if (virCgroupGetCpuacctPercpuUsage(priv-cgroup, buf))
+if (virCgroupGetCpuacctPercpuUsage(group, buf))
goto cleanup;
pos = buf;
@@ -16113,7 +16113,7 @@ qemuDomainGetPercpuStats(virDomainObjPtr vm,
if (VIR_ALLOC_N(sum_cpu_time, n) 0)
goto cleanup;
-if (getSumVcpuPercpuStats(vm, sum_cpu_time, n) 0)
+if (getSumVcpuPercpuStats(group, nvcpupids, sum_cpu_time, n) 0)
goto cleanup;
sum_cpu_pos = sum_cpu_time;
@@ -16177,8 +16177,8 @@ qemuDomainGetCPUStats(virDomainPtr domain,
ret = virCgroupGetDomainTotalCpuStats(priv-cgroup,
params, nparams);
else
-ret = qemuDomainGetPercpuStats(vm, params, nparams,
- start_cpu, ncpus);
+ret = qemuDomainGetPercpuStats(priv-cgroup, params, nparams,
+ start_cpu, ncpus, priv-nvcpupids);
cleanup:
if (vm)
virObjectUnlock(vm);
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/6] Rename id, max_id to need_cpus, total_cpus
total_cpus is the total number of CPUs on the host
need_cpus is the number of CPUs we need to look at
(need_cpus can be larger than ncpus, because we need to look
at CPUs before the startcpu too, even if we aren't reporting
their stats)
---
src/util/vircgroup.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 7a7f52b..de57276 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -2900,7 +2900,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
{
int rv = -1;
size_t i;
-int id, max_id;
+int need_cpus, total_cpus;
char *pos;
char *buf = NULL;
unsigned long long *sum_cpu_time = NULL;
@@ -2919,19 +2919,19 @@ virCgroupGetPercpuStats(virCgroupPtr group,
}
/* To parse account file, we need to know how many cpus are present. */
-max_id = nodeGetCPUCount();
-if (max_id 0)
+total_cpus = nodeGetCPUCount();
+if (total_cpus 0)
return rv;
-if (ncpus == 0) { /* returns max cpu ID */
-rv = max_id;
+if (ncpus == 0) {
+rv = total_cpus;
goto cleanup;
}
-if (start_cpu max_id) {
+if (start_cpu total_cpus) {
virReportError(VIR_ERR_INVALID_ARG,
_(start_cpu %d larger than maximum of %d),
- start_cpu, max_id);
+ start_cpu, total_cpus);
goto cleanup;
}
@@ -2944,12 +2944,12 @@ virCgroupGetPercpuStats(virCgroupPtr group,
param_idx = 0;
/* number of cpus to compute */
-if (start_cpu = max_id - ncpus)
-id = max_id - 1;
+if (start_cpu = total_cpus - ncpus)
+need_cpus = total_cpus - 1;
else
-id = start_cpu + ncpus - 1;
+need_cpus = start_cpu + ncpus - 1;
-for (i = 0; i = id; i++) {
+for (i = 0; i = need_cpus; i++) {
if (virStrToLong_ull(pos, pos, 10, cpu_time) 0) {
virReportError(VIR_ERR_INTERNAL_ERROR, %s,
_(cpuacct parse error));
@@ -2976,7 +2976,7 @@ virCgroupGetPercpuStats(virCgroupPtr group,
goto cleanup;
sum_cpu_pos = sum_cpu_time;
-for (i = 0; i = id; i++) {
+for (i = 0; i = need_cpus; i++) {
cpu_time = *(sum_cpu_pos++);
if (i start_cpu)
continue;
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/6] Check maximum startcpu value correctly
The cpus are indexed from 0, so a startcpu value equal
to the number of CPUs is invalid.
https://bugzilla.redhat.com/show_bug.cgi?id=1070680
---
src/util/vircgroup.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index de57276..2272bc6 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -2928,10 +2928,10 @@ virCgroupGetPercpuStats(virCgroupPtr group,
goto cleanup;
}
-if (start_cpu total_cpus) {
+if (start_cpu = total_cpus) {
virReportError(VIR_ERR_INVALID_ARG,
_(start_cpu %d larger than maximum of %d),
- start_cpu, total_cpus);
+ start_cpu, total_cpus - 1);
goto cleanup;
}
--
1.8.3.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add V6LOCAL parameter to nwfilter rules.
On Wed, Apr 02, 2014 at 03:40:14PM -0400, Brian Rak wrote:
@@ -163,6 +164,28 @@
virNWFilterVarHashmapAddStdValues(virNWFilterHashTablePtr table,
%s, _(Could not add variable 'MAC' to
hashmap));
return -1;
}
+
+virMacAddr parsedMac;
+if (virMacAddrParse(macaddr, parsedMac) == 0)
+{
+parsedMac.addr[0] ^= 2;
+
+char euiMacAddr[26];
+snprintf(euiMacAddr, sizeof(euiMacAddr),
fe80::%x%x:%xff:fe%x:%x%x, parsedMac.addr[0], parsedMac.addr[1],
parsedMac.addr[2],
+parsedMac.addr[3], parsedMac.addr[4], parsedMac.addr[5]);
Unfortunately this IPv6 addressing scheme isn't portable. Windows Vista
will create link local addresses which are completely random, not based
on the MAC address.
http://technet.microsoft.com/en-us/magazine/2007.08.cableguy.aspx
I wonder if there's a way todo something more clever for IPv6 to learn
the addresses, we as do for IPv4 address learning, or snoop route
advertisment traffic as we do for DHCP
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] Add V6LOCAL nwfilter parameter.
On Thu, Apr 03, 2014 at 01:51:20AM -0400, Brian Rak wrote: Currently, adding any sort of IPv6 nwfilter rules is rather difficult. There are no standard rules, and you end up doing a lot of things by hand. This patch makes the $V6LOCAL variable available within nwfilter nules. This is the generated from the interface's mac address using the modified EUI-64 format, which matches what the guest should be using. See my previous response about this not being portable - guests are not required to use the MAC address in link local addressing. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 1/5] Introduce virDomainFSFreeze() and virDomainFSThaw() public API
On Thu, Apr 03, 2014 at 11:39:29AM -0400, Tomoki Sekiyama wrote: These will freeze and thaw filesystems within guest. The APIs take @disks and @ndisks parameters to specify disks to be frozen or thawed. The parameters can be NULL and 0, then the all mounted filesystes are frozen or thawed. If some disks are frozen multiple times, they are not thawed until requested to be thawed as many times as freeze request. @flags parameter, which are currently not used, is for future extensions. Signed-off-by: Tomoki Sekiyama tomoki.sekiy...@hds.com --- include/libvirt/libvirt.h.in | 10 + src/driver.h | 14 ++ src/libvirt.c| 92 ++ src/libvirt_public.syms |6 +++ 4 files changed, 122 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 930b7e8..d408f19 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5277,6 +5277,16 @@ int virDomainFSTrim(virDomainPtr dom, unsigned long long minimum, unsigned int flags); +int virDomainFSFreeze(virDomainPtr dom, + const char **disks, + unsigned int ndisks, + unsigned int flags); + +int virDomainFSThaw(virDomainPtr dom, +const char **disks, +unsigned int ndisks, +unsigned int flags); Are all guests OS required to support unfreeze on a per disk basis. I vaguely recall someone mentioning that some OS can do all-or-none only. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5] tests: refactor virstoragetest for less stack space
On 04/04/14 06:32, Eric Blake wrote: I'm about to add fields to virStorageFileMetadata, which means also adding fields to the testFileData struct in virstoragetest. Alas, adding even one pointer on an x86_64 machine gave me a dreaded compiler error: virstoragetest.c:712:1: error: the frame size of 4208 bytes is larger than 4096 bytes [-Werror=frame-larger-than=] After some experimentation, I realized that each test was creating yet another testChainData (which contains testFileData) on the stack; forcing the reuse of one of these structures instead of creating a fresh one each time drastically reduces the size requirements. While at it, I also got rid of a lot of intermediate structs, with some macro magic that lets me directly build up the destination chains inline. Unfortunately it's not completely trivial to understand what's happening on the first glance. * tests/virstoragetest.c (mymain): Rewrite TEST_ONE_CHAIN to reuse the same struct for each test, and to take the data inline rather than via intermediate variables. (testChainData): Use bounded array of pointers instead of unlimited array of struct. (testStorageChain): Reflect struct change. Signed-off-by: Eric Blake ebl...@redhat.com --- tests/virstoragetest.c | 167 - 1 file changed, 81 insertions(+), 86 deletions(-) As it's test code: ACK Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Enhancing clean-traffic to work with IPv6
On Thu, Apr 03, 2014 at 05:28:35PM -0400, Brian Rak wrote: I'm looking into adding IPv6 support to the nwfilter clean-traffic rules, but I'm unsure of the best approach to this. I'm planning on sending patches once I get this correct, so I'm trying to figure out what way fits in best. There's a couple different ways I can think of: 1) Explicitly add v6 rules to the existing clean-traffic rules. This would enable IPv6 for guests whenever libvirt was upgraded, which may be a problem. 2) Add another filter chain (clean-ipv6-traffic) that would do the same thing as clean-traffic, just for IPv6 3) Add another filter chain (clean-ipv6-ipv4-traffic), that would clean IPv6 traffic, and include the clean-traffic filter set The limitation here is that IP learning will not work for IPv6, so actually using IPv6 is going to require passing in parameters to filter specifying what ranges the guest should be allowed to use. I think this rules out #1. Why do you say IP learning won't work ? The current impl of IP learning only supports IPv4, but AFAIK, it should be viable to enhance it to detect an address from the first outbound IPv6 packet, or by snooping DHCPv6 responses, just as we do for IPv4 Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5] conf: track when storage type is still undetermined
On 04/04/14 06:32, Eric Blake wrote: Right now, virStorageFileMetadata tracks bool backingStoreIsFile for whether the backing string specified in metadata can be resolved as a file (covering both block and regular file resources) or is treated as a network protocol. But when merging this struct with virStorageSource, it will be easier to just actually track which type of resource it is, as well as have a reserved value for the case where the resource type is unknown (or had an error during probing). * src/util/virstoragefile.h (virStorageType): Add a placeholder value, swap order to match similar public enum. * src/util/virstoragefile.c (virStorage): Update string mapping. * src/conf/domain_conf.c (virDomainDiskSourceParse) (virDomainDiskDefParseXML, virDomainDiskDefFormat) (virDomainDiskSourceFormat): Adjust clients. * src/conf/snapshot_conf.c (virDomainSnapshotDiskDefParseXML): Likewise. * src/qemu/qemu_driver.c (qemuDomainSnapshotPrepareDiskExternalBackingInactive) (qemuDomainSnapshotPrepareDiskExternalOverlayActive) (qemuDomainSnapshotPrepareDiskExternalOverlayInactive) (qemuDomainSnapshotPrepareDiskInternal) (qemuDomainSnapshotCreateSingleDiskActive): Likewise. * src/qemu/qemu_command.c (qemuGetDriveSourceString): Likewise. Signed-off-by: Eric Blake ebl...@redhat.com --- src/conf/domain_conf.c| 10 ++ src/conf/snapshot_conf.c | 2 +- src/qemu/qemu_command.c | 1 + src/qemu/qemu_driver.c| 11 +-- src/util/virstoragefile.c | 3 ++- src/util/virstoragefile.h | 8 ++-- 6 files changed, 25 insertions(+), 10 deletions(-) ACK, having _TYPE_BLOCK as index 0 caused grief a lot of times for me. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/7] virsh: Make vshCommandOpt* report error
On 04/02/2014 02:43 PM, Michal Privoznik wrote:
Currently, the virsh code is plenty of the following pattern:
if (vshCommandOptUInt(..) 0) {
vshError(...);
goto cleanup;
}
It doesn't make much sense to repeat the code everywhere.
Moreover, some functions from the family already report
error some of them don't.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/vcpupin | 1 +
tools/virsh.c | 72
+--
vshCommandOptTimeoutToMs also calls vshCommandOptInt
tools/virsh.h | 2 +-
3 files changed, 67 insertions(+), 8 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index f1fb038..a616216 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -34,6 +34,7 @@ fail=0
$abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1
out 21
test $? = 1 || fail=1
cat \EOF exp || fail=1
+error: Unable to parse integer parameter to --vcpu
error: vcpupin: Invalid or missing vCPU number.
EOF
diff --git a/tools/virsh.c b/tools/virsh.c
index f2e4c4a..1371abb 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1489,8 +1489,18 @@ vshCommandOptInt(const vshCmd *cmd, const char *name,
int *value)
if (ret = 0)
return ret;
-if (virStrToLong_i(arg-data, NULL, 10, value) 0)
+if (virStrToLong_i(arg-data, NULL, 10, value) 0) {
+if (arg-def-flags VSH_OFLAG_REQ) {
+vshError(NULL,
ctl should be used for reporting errors, just like in vshCommandOptStringReq.
+ _(missing --%s parameter value),
+ name);
Missing values are caught in vshCommandParse. The error should be the same
regardless of VSH_OFLAG_REQ.
+} else {
+vshError(NULL,
+ _(Unable to parse integer parameter to --%s),
+ name);
@@ -1692,9 +1742,17 @@ vshCommandOptScaledInt(const vshCmd *cmd, const char
*name,
ret = vshCommandOptString(cmd, name, str);
if (ret = 0)
return ret;
-if (virStrToLong_ull(str, end, 10, value) 0 ||
-virScaleInteger(value, end, scale, max) 0)
+if (virStrToLong_ull(str, end, 10, value) 0) {
+vshError(NULL,
+ _(Unable to parse integer parameter to --%s),
+ name);
return -1;
+}
+
+if (virScaleInteger(value, end, scale, max) 0) {
+/* Error reported by the helper. */
Needs vshReportError to propagate the error.
+return -1;
+}
return 1;
}
diff --git a/tools/virsh.h b/tools/virsh.h
index 3e0251b..6eb17b3 100644
--- a/tools/virsh.h
+++ b/tools/virsh.h
@@ -168,7 +168,7 @@ struct _vshCmdInfo {
struct _vshCmdOptDef {
const char *name; /* the name of option, or NULL for list end
*/
vshCmdOptType type; /* option type */
-unsigned int flags; /* flags */
+unsigned int flags; /* bitwise OR of VSH_OFLAG_**/
const char *help; /* non-NULL help string; or for VSH_OT_ALIAS
* the name of a later public option */
vshCompleter completer; /* option completer */
You can push this hunk separately as trivial.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] virsh-domain: Adapt to new error reporting
On 04/02/2014 02:43 PM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/vcpupin| 1 -
tools/virsh-domain.c | 75
+---
2 files changed, 24 insertions(+), 52 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index a616216..638f62b 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -35,7 +35,6 @@ $abs_top_builddir/tools/virsh --connect test:///default
vcpupin test a 0,1 out
test $? = 1 || fail=1
cat \EOF exp || fail=1
error: Unable to parse integer parameter to --vcpu
-error: vcpupin: Invalid or missing vCPU number.
EOF
compare exp out || fail=1
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 73414f8..6e1eb17 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1120,7 +1120,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
if ((rv = vshCommandOptULongLong(cmd, total-bytes-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
@@ -1129,7 +1129,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, read-bytes-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
@@ -1138,7 +1138,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, write-bytes-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
@@ -1147,7 +1147,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, total-iops-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
@@ -1156,7 +1156,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, read-iops-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
@@ -1165,7 +1165,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, write-iops-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC,
@@ -1216,10 +1216,6 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
error:
vshError(ctl, %s, _(Unable to change block I/O throttle));
goto cleanup;
-
- interror:
-vshError(ctl, %s, _(Unable to parse integer parameter));
-goto cleanup;
}
These should jump to cleanup, not error.
Jan
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] libxl: Implement basic video device selection
Ok, I think indentation should be ok like that and I added error
handling for VIR_STRDUP. I think just returning -1 like the other
places should be ok as the only caller looks to handle the disposal
of the config structure.
-Stefan
---
From dfe579003e91137ecd824d2a08bcdc8f18725857 Mon Sep 17 00:00:00 2001
From: Stefan Bader stefan.ba...@canonical.com
Date: Thu, 27 Mar 2014 16:01:18 +0100
Subject: [PATCH] libxl: Implement basic video device selection
This started as an investigation into an issue where libvirt (using the
libxl driver) and the Xen host, like an old couple, could not agree on
who is responsible for selecting the VNC port to use.
Things usually (and a bit surprisingly) did work because, just like that
old couple, they had the same idea on what to do by default. However it
was possible that this ended up in a big argument.
The problem is that display information exists in two different places:
in the vfbs list and in the build info. And for launching the device model,
only the latter is used. But that never gets initialized from libvirt. So
Xen allows the device model to select a default port while libvirt thinks
it has told Xen that this is done by libvirt (though the vfbs config).
While fixing that, I made a stab at actually evaluating the configuration
of the video device. So that it is now possible to at least decide between
a Cirrus or standard VGA emulation and to modify the VRAM within certain
limits using libvirt.
Signed-off-by: Stefan Bader stefan.ba...@canonical.com
---
src/libxl/libxl_conf.c | 93
1 file changed, 93 insertions(+)
diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c
index b8de72a..042f4da 100644
--- a/src/libxl/libxl_conf.c
+++ b/src/libxl/libxl_conf.c
@@ -1298,6 +1298,89 @@ libxlMakeCapabilities(libxl_ctx *ctx)
return NULL;
}
+static int
+libxlSetBuildGraphics(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+libxl_domain_build_info *b_info = d_config-b_info;
+
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if less
+ * is specified.
+ */
+switch (b_info-device_model_version) {
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+min_vram = 8 * 1024;
+break;
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+default:
+min_vram = 16 * 1024;
+}
+break;
+default:
+/*
+ * Ignore any other device type and use Cirrus. Again fix
+ * up the minimal VRAM to what libxl expects.
+ */
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+switch (b_info-device_model_version) {
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+min_vram = 4 * 1024; /* Actually the max, too */
+break;
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+default:
+min_vram = 8 * 1024;
+}
+}
+b_info-video_memkb = (def-videos[0]-vram min_vram) ?
+ def-videos[0]-vram :
+ LIBXL_MEMKB_DEFAULT;
+} else {
+libxl_defbool_set(b_info-u.hvm.nographic, 1);
+}
+
+/*
+ * When making the list of displays, only VNC and SDL types were
+ * taken into account. So it seems sensible to connect the default
+ * video device to the first in the vfb list.
+ *
+ * FIXME: Copy the structures and fixing the strings feels a bit dirty.
+ */
+if (d_config-num_vfbs) {
+libxl_device_vfb *vfb0 = d_config-vfbs[0];
+
+b_info-u.hvm.vnc = vfb0-vnc;
+if (VIR_STRDUP(b_info-u.hvm.vnc.listen, vfb0-vnc.listen) 0)
+return -1;
+if (VIR_STRDUP(b_info-u.hvm.vnc.passwd, vfb0-vnc.passwd) 0)
+return -1;
+b_info-u.hvm.sdl = vfb0-sdl;
+if (VIR_STRDUP(b_info-u.hvm.sdl.display, vfb0-sdl.display) 0)
+return -1;
+if
Re: [libvirt] [PATCH 4/5] conf: track more fields in backing chain metadata
On 04/04/14 06:32, Eric Blake wrote:
The current use of virStorageFileMetadata is awkward; to learn
some of the information about a child node, you have to read
fields in the parent node. This does not lend itself well to
modifying backing chains (whether inserting a new node in the
chain, or consolidating existing nodes); better would be to
learn about a child node directly in that node. This patch
sets up some new fields which contain redundant information,
although not necessarily in the final desired state for the
new fields (see the next patch for actual tests of what is there
now). Then later patches will do any refactoring necessary to
get the fields to their desired states, and update clients to
get the information from the new fields, so we can finally
delete the fields that are tracking information about the wrong
node.
* src/util/virstoragefile.h (_virStorageFileMetadata): Add
path, canonName, relDir, type, and format fields. Reorder
existing fields, and add lots of comments.
* src/util/virstoragefile.c (virStorageFileFreeMetadata): Clean
new fields.
(virStorageFileGetMetadataInternal)
(virStorageFileGetMetadataFromFDInternal): Start populating new
fields.
Signed-off-by: Eric Blake ebl...@redhat.com
---
src/util/virstoragefile.c | 30 --
src/util/virstoragefile.h | 35 ++-
2 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 0e1461d..d741fb7 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
...
return ret;
@@ -1205,6 +1227,10 @@ virStorageFileFreeMetadata(virStorageFileMetadata
*meta)
if (!meta)
return;
+VIR_FREE(meta-path);
+VIR_FREE(meta-canonName);
+VIR_FREE(meta-relDir);
+
virStorageFileFreeMetadata(meta-backingMeta);
VIR_FREE(meta-backingStore);
VIR_FREE(meta-backingStoreRaw);
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 3807285..a284e37 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -112,17 +112,42 @@ struct _virStorageTimestamps {
typedef struct _virStorageFileMetadata virStorageFileMetadata;
typedef virStorageFileMetadata *virStorageFileMetadataPtr;
struct _virStorageFileMetadata {
-char *backingStore; /* Canonical name (absolute file, or protocol) */
-char *backingStoreRaw; /* If file, original name, possibly relative */
-char *directory; /* The directory containing basename of backingStoreRaw
*/
-int backingStoreFormat; /* enum virStorageFileFormat */
-bool backingStoreIsFile;
+/* Name of this file as spelled by the user (top level) or
+ * metadata of the parent (if this is a backing store). */
+char *path;
+/* Canonical name of this file, used to detect loops in the
+ * backing store chain. */
+char *canonName;
+/* Directory to start from if backingStoreRaw is a relative file
+ * name */
+char *relDir;
+/* Name of the backing store recorded in metadata of the parent */
+char *backingStoreRaw;
Hmm, this field seems pretty redundant to me, IIUC it's the same data as
in path.
OTOH, the path field should contain the canonical path as the target
is to convert it to the new storage file struct. In that case the
canonName will be redundant and backingStoreRaw needs to be kept to
track the original name.
In any case ... one of them seems to be duplicate.
+
+/* Backing chain. backingMeta-origName should match
+ * backingStoreRaw; but the fields are duplicated in the common
+ * case to make it possible to detect missing backing files, as
+ * follows: if backingStoreRaw is NULL, this should be NULL. If
+ * this is NULL and backingStoreRaw is non-NULL, there was an
+ * error following the chain (such as missing file). Otherwise,
+ * information about the backing store is here. */
virStorageFileMetadataPtr backingMeta;
+/* Details about the current image */
+int type; /* enum virStorageType */
+int format; /* enum virStorageFileFormat */
virStorageEncryptionPtr encryption;
unsigned long long capacity;
virBitmapPtr features; /* bits described by enum virStorageFileFeature */
char *compat;
+
+/* Fields I'm trying to delete, because it is confusing to have to
+ * query the parent metadata for details about the backing
+ * store. */
+char *backingStore; /* Canonical name (absolute file, or protocol).
Should be same as backingMeta-canon */
+char *directory; /* The directory containing basename of
backingStoreRaw. Should be same as backingMeta-relDir */
+int backingStoreFormat; /* enum virStorageFileFormat. Should be same as
backingMeta-format */
+bool backingStoreIsFile; /* Should be same as backingMeta-type
VIR_STORAGE_TYPE_NETWORK */
};
Peter
signature.asc
Re: [libvirt] [PATCH v2] libxl: Implement basic video device selection
On Fri, 2014-04-04 at 11:36 +0200, Stefan Bader wrote:
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if less
+ * is specified.
Build failures? Do you mean domain build rather than libvirt build?
I'm not sure about this fixing up the GIGO from the user side, but
that's a libvirt policy decision I suppose? If it were me I would just
let libxl provide the error and expect people to fix their domain config
rather than silently giving them something other than what they asked
for. What if increasing the VRAM causes a cascading failure due e.g. to
lack of memory? That's going to be tricky to debug I think!
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5] conf: start testing contents of the new backing chain fields
On 04/04/14 06:32, Eric Blake wrote: The testsuite is absolutely essential to feeling comfortable about swapping the backing chain structure over to a new format. This patch tests the path settings, and demonstrates that right now the code is passing only the canonical name to the child struct, which means more work is necessary in virstoragefile to pass the user spelling alongside the canonical name down to the child. * tests/virstoragetest.c (testStorageChain): Test path. (mymain): Update expected data. Signed-off-by: Eric Blake ebl...@redhat.com --- tests/virstoragetest.c | 239 - 1 file changed, 175 insertions(+), 64 deletions(-) Looks reasonable. ACK. Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libxl: Implement basic video device selection
On Fri, 2014-04-04 at 12:31 +0200, Stefan Bader wrote:
On 04.04.2014 11:48, Ian Campbell wrote:
On Fri, 2014-04-04 at 11:36 +0200, Stefan Bader wrote:
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if less
+ * is specified.
Build failures? Do you mean domain build rather than libvirt build?
I'm not sure about this fixing up the GIGO from the user side, but
that's a libvirt policy decision I suppose? If it were me I would just
let libxl provide the error and expect people to fix their domain config
rather than silently giving them something other than what they asked
for. What if increasing the VRAM causes a cascading failure due e.g. to
lack of memory? That's going to be tricky to debug I think!
In the end its a start a domain with such a config. Which seems to be what I
would end up with in my testing with an admittedly older version of
virt-manager
connecting to a libvirtd running an initial version of this patch without
that part.
The error seen on the front-end was something along the lines of failed to
get
enough memory to start the guest (the libxl log on the other side had the
better error message). And the gui always reduces the memory below the minimum
for both the options (VGA and Xen).
That is the reason I went for meh, go for the minimum anyways.
Does the libvirt protocol require the client to provide all the sizes
etc with no provision for asking the server side to pick a sane default?
And btw, it is already confusing enough as with cirrus, I get a 9M by default
which is passed on to qemu on the command line and then ignored by it and one
gets 32M in any way...
Lovely!
Ian.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Xen-devel] [PATCH v2] libxl: Implement basic video device selection
On 04.04.2014 12:31, Stefan Bader wrote:
On 04.04.2014 11:48, Ian Campbell wrote:
On Fri, 2014-04-04 at 11:36 +0200, Stefan Bader wrote:
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if less
+ * is specified.
Build failures? Do you mean domain build rather than libvirt build?
I'm not sure about this fixing up the GIGO from the user side, but
that's a libvirt policy decision I suppose? If it were me I would just
let libxl provide the error and expect people to fix their domain config
rather than silently giving them something other than what they asked
for. What if increasing the VRAM causes a cascading failure due e.g. to
lack of memory? That's going to be tricky to debug I think!
In the end its a start a domain with such a config. Which seems to be what I
*sigh* That wasn't much better English. Yes, I meant a domain build or a failure
to start a domain...
would end up with in my testing with an admittedly older version of
virt-manager
connecting to a libvirtd running an initial version of this patch without
that part.
The error seen on the front-end was something along the lines of failed to
get
enough memory to start the guest (the libxl log on the other side had the
better error message). And the gui always reduces the memory below the minimum
for both the options (VGA and Xen).
That is the reason I went for meh, go for the minimum anyways.
And btw, it is already confusing enough as with cirrus, I get a 9M by default
which is passed on to qemu on the command line and then ignored by it and one
gets 32M in any way...
-Stefan
___
Xen-devel mailing list
xen-de...@lists.xen.org
http://lists.xen.org/xen-devel
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libxl: Implement basic video device selection
On 04.04.2014 12:34, Ian Campbell wrote:
On Fri, 2014-04-04 at 12:31 +0200, Stefan Bader wrote:
On 04.04.2014 11:48, Ian Campbell wrote:
On Fri, 2014-04-04 at 11:36 +0200, Stefan Bader wrote:
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if less
+ * is specified.
Build failures? Do you mean domain build rather than libvirt build?
I'm not sure about this fixing up the GIGO from the user side, but
that's a libvirt policy decision I suppose? If it were me I would just
let libxl provide the error and expect people to fix their domain config
rather than silently giving them something other than what they asked
for. What if increasing the VRAM causes a cascading failure due e.g. to
lack of memory? That's going to be tricky to debug I think!
In the end its a start a domain with such a config. Which seems to be what I
would end up with in my testing with an admittedly older version of
virt-manager
connecting to a libvirtd running an initial version of this patch without
that part.
The error seen on the front-end was something along the lines of failed to
get
enough memory to start the guest (the libxl log on the other side had the
better error message). And the gui always reduces the memory below the
minimum
for both the options (VGA and Xen).
That is the reason I went for meh, go for the minimum anyways.
Does the libvirt protocol require the client to provide all the sizes
etc with no provision for asking the server side to pick a sane default?
I think libvirt itself would be ok with a config that does not enforce a certain
amount of VRAM (libvirt devs correct me if I am talking non-sense here).
But together with the virt-manager front-end that tries to be helpful halfway.
I get a incorrect setting which I cannot change from that version of the gui.
Again, never versions might be better.
Just practically I expect some crazy people (like me *cough*) to try this from
an older Desktop release...
And btw, it is already confusing enough as with cirrus, I get a 9M by default
which is passed on to qemu on the command line and then ignored by it and one
gets 32M in any way...
Lovely!
Ian.
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v5 1/5] Introduce virDomainFSFreeze() and virDomainFSThaw() public API
On 04/04/2014 02:51 AM, Daniel P. Berrange wrote: On Thu, Apr 03, 2014 at 11:39:29AM -0400, Tomoki Sekiyama wrote: These will freeze and thaw filesystems within guest. The APIs take @disks and @ndisks parameters to specify disks to be frozen or thawed. The parameters can be NULL and 0, then the all mounted filesystes are frozen or thawed. If some disks are frozen multiple times, they are not thawed until requested to be thawed as many times as freeze request. @flags parameter, which are currently not used, is for future extensions. +int virDomainFSThaw(virDomainPtr dom, +const char **disks, +unsigned int ndisks, +unsigned int flags); Are all guests OS required to support unfreeze on a per disk basis. I vaguely recall someone mentioning that some OS can do all-or-none only. If that's the case, the command can issue an error if disks is non-NULL but the guest agent required an all-or-none operation. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 0/2] Honor DAC norelabel attribute
Version two, this time split into two patches. Michal Privoznik (2): security_dac: Rework to make it more readable security_dac: Honor norelabel attribute src/security/security_dac.c | 355 ++-- 1 file changed, 213 insertions(+), 142 deletions(-) -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 1/2] security_dac: Rework to make it more readable
While going through the security DAC code, I realized
it's a ragbag with plenty of thing we try to avoid.
For instance, callback data are passed as:
void params[2];
params[0] = mgr;
params[1] = def;
Moreover, there's no need to pass the whole
virDomainDef in the callback as the only thing needed
in the callbacks is virSecurityLabelDefPtr. Okay, in
some cases the callbacks report error with a domain
name, but that can be changed.
The other thing, in switch() we ought use enum types
as it is much safer when adding a new item to the
enum.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/security/security_dac.c | 271 +++-
1 file changed, 144 insertions(+), 127 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 8512767..8835d49 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -53,6 +53,15 @@ struct _virSecurityDACData {
char *baselabel;
};
+typedef struct _virSecurityDACCallbackData virSecurityDACCallbackData;
+typedef virSecurityDACCallbackData *virSecurityDACCallbackDataPtr;
+
+struct _virSecurityDACCallbackData {
+virSecurityManagerPtr manager;
+virSecurityLabelDefPtr secdef;
+};
+
+
/* returns -1 on error, 0 on success */
int
virSecurityDACSetUserAndGroup(virSecurityManagerPtr mgr,
@@ -81,65 +90,42 @@ virSecurityDACSetDynamicOwnership(virSecurityManagerPtr mgr,
/* returns 1 if label isn't found, 0 on success, -1 on error */
static int
-virSecurityDACParseIds(virDomainDefPtr def, uid_t *uidPtr, gid_t *gidPtr)
+virSecurityDACParseIds(virSecurityLabelDefPtr seclabel,
+ uid_t *uidPtr, gid_t *gidPtr)
{
-uid_t uid;
-gid_t gid;
-virSecurityLabelDefPtr seclabel;
-
-if (def == NULL)
-return 1;
-
-seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (seclabel == NULL || seclabel-label == NULL) {
-VIR_DEBUG(DAC seclabel for domain '%s' wasn't found, def-name);
+if (!seclabel || !seclabel-label)
return 1;
-}
-if (virParseOwnershipIds(seclabel-label, uid, gid) 0)
+if (virParseOwnershipIds(seclabel-label, uidPtr, gidPtr) 0)
return -1;
-if (uidPtr)
-*uidPtr = uid;
-if (gidPtr)
-*gidPtr = gid;
-
return 0;
}
static int
-virSecurityDACGetIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
+virSecurityDACGetIds(virSecurityLabelDefPtr seclabel,
+ virSecurityDACDataPtr priv,
uid_t *uidPtr, gid_t *gidPtr,
gid_t **groups, int *ngroups)
{
int ret;
-if (!def !priv) {
-virReportError(VIR_ERR_INTERNAL_ERROR, %s,
- _(Failed to determine default DAC seclabel
- for an unknown object));
-return -1;
-}
-
if (groups)
*groups = priv ? priv-groups : NULL;
if (ngroups)
*ngroups = priv ? priv-ngroups : 0;
-if ((ret = virSecurityDACParseIds(def, uidPtr, gidPtr)) = 0)
+if ((ret = virSecurityDACParseIds(seclabel, uidPtr, gidPtr)) = 0)
return ret;
if (!priv) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _(DAC seclabel couldn't be determined
- for domain '%s'), def-name);
+virReportError(VIR_ERR_INTERNAL_ERROR, %s,
+ _(DAC seclabel couldn't be determined));
return -1;
}
-if (uidPtr)
-*uidPtr = priv-user;
-if (gidPtr)
-*gidPtr = priv-group;
+*uidPtr = priv-user;
+*gidPtr = priv-group;
return 0;
}
@@ -147,60 +133,36 @@ virSecurityDACGetIds(virDomainDefPtr def,
virSecurityDACDataPtr priv,
/* returns 1 if label isn't found, 0 on success, -1 on error */
static int
-virSecurityDACParseImageIds(virDomainDefPtr def,
+virSecurityDACParseImageIds(virSecurityLabelDefPtr seclabel,
uid_t *uidPtr, gid_t *gidPtr)
{
-uid_t uid;
-gid_t gid;
-virSecurityLabelDefPtr seclabel;
-
-if (def == NULL)
-return 1;
-
-seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
-if (seclabel == NULL || seclabel-imagelabel == NULL) {
-VIR_DEBUG(DAC imagelabel for domain '%s' wasn't found, def-name);
+if (!seclabel || !seclabel-imagelabel)
return 1;
-}
-if (virParseOwnershipIds(seclabel-imagelabel, uid, gid) 0)
+if (virParseOwnershipIds(seclabel-imagelabel, uidPtr, gidPtr) 0)
return -1;
-if (uidPtr)
-*uidPtr = uid;
-if (gidPtr)
-*gidPtr = gid;
-
return 0;
}
static int
-virSecurityDACGetImageIds(virDomainDefPtr def, virSecurityDACDataPtr priv,
+virSecurityDACGetImageIds(virSecurityLabelDefPtr seclabel,
+ virSecurityDACDataPtr priv,
uid_t *uidPtr, gid_t *gidPtr)
{
int ret;
-if (!def !priv) {
-
Re: [libvirt] [PATCH 2/5] tests: refactor virstoragetest for less stack space
On 04/04/2014 02:54 AM, Peter Krempa wrote:
On 04/04/14 06:32, Eric Blake wrote:
I'm about to add fields to virStorageFileMetadata, which means
also adding fields to the testFileData struct in virstoragetest.
Alas, adding even one pointer on an x86_64 machine gave me a
dreaded compiler error:
virstoragetest.c:712:1: error: the frame size of 4208 bytes is larger than
4096 bytes [-Werror=frame-larger-than=]
After some experimentation, I realized that each test was creating
yet another testChainData (which contains testFileData) on the stack;
forcing the reuse of one of these structures instead of creating a
fresh one each time drastically reduces the size requirements. While
at it, I also got rid of a lot of intermediate structs, with some
macro magic that lets me directly build up the destination chains
inline.
Unfortunately it's not completely trivial to understand what's happening
on the first glance.
I'll expand the commit message before pushing. Basically,
-#define TEST_ONE_CHAIN(id, start, format, chain, flags) \
+#define TEST_ONE_CHAIN(id, start, format, flags, ...)\
do { \
-struct testChainData data = {\
-start, format, chain, ARRAY_CARDINALITY(chain), flags, \
the old code was populating data.files = chain, where 'chain' was an
intermediate variable and files is a testFileData*
+size_t i;\
+memset(data, 0, sizeof(data)); \
+data = (struct testChainData){ \
+start, format, { __VA_ARGS__ }, 0, flags,\
}; \
while the new code is populating data.files = { ptr, ptr, ... }, with no
intermediate chain variable, and where files is now an array of
testFileData*.
+for (i = 0; i ARRAY_CARDINALITY(data.files); i++) \
+if (data.files[i]) \
+data.nfiles++; \
Pre-patch, counting the number of files was done by the size of the
'chain' intermediate variable. Post-patch, it is done by counting the
number of non-NULL pointers in the 'files' array.
+TEST_ONE_CHAIN(#id a, relstart, format, flags1,\
+ VIR_FLATTEN_1(chain1)); \
Here, VIR_FLATTEN_1 is macro magic that takes a single macro argument in
the form '(a, b, c)' and turns it into multiple arguments 'a, b, c' to
TEST_ONE_CHAIN's use of '...'
/* Raw image, whether with right format or no specified format */
-const testFileData chain1[] = { raw };
TEST_CHAIN(1, raw, absraw, VIR_STORAGE_FILE_RAW,
- chain1, EXP_PASS,
- chain1, ALLOW_PROBE | EXP_PASS,
- chain1, EXP_PASS,
- chain1, ALLOW_PROBE | EXP_PASS);
+ (raw), EXP_PASS,
+ (raw), ALLOW_PROBE | EXP_PASS,
+ (raw), EXP_PASS,
+ (raw), ALLOW_PROBE | EXP_PASS);
Then for each test, I replaced uses of 'chain1' (the intermediate
variable)' with '(contents of chain1)' as a direct argument to TEST_CHAIN.
As it's test code: ACK
Yes, we have that going for us - if it compiles and still passes the
testsuite, it was probably correct :)
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2 2/2] security_dac: Honor norelabel attribute
The inspiration for this patch comes from a question on the list
asking if there's a way to not label some disks. Well, in DAC driver
there's not. Even if user have requested norelabel:
disk type='file' device='disk'
driver name='qemu' type='raw'/
source file='/some/dummy/path/test.bin'
seclabel model='dac' relabel='no'/
/source
target dev='vdb' bus='virtio'/
readonly/
address type='pci' domain='0x' bus='0x00' slot='0x07'
function='0x0'/
/disk
the DAC driver ignores this completely.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
src/security/security_dac.c | 92 +++--
1 file changed, 73 insertions(+), 19 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 8835d49..f15a0e9 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -286,7 +286,7 @@ virSecurityDACRestoreSecurityFileLabel(const char *path)
static int
-virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk ATTRIBUTE_UNUSED,
+virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr disk,
const char *path,
size_t depth ATTRIBUTE_UNUSED,
void *opaque)
@@ -295,11 +295,23 @@ virSecurityDACSetSecurityFileLabel(virDomainDiskDefPtr
disk ATTRIBUTE_UNUSED,
virSecurityManagerPtr mgr = cbdata-manager;
virSecurityLabelDefPtr secdef = cbdata-secdef;
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virSecurityDeviceLabelDefPtr disk_seclabel;
uid_t user;
gid_t group;
-if (virSecurityDACGetImageIds(secdef, priv, user, group) 0)
-return -1;
+disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk,
+SECURITY_DAC_NAME);
+
+if (disk_seclabel disk_seclabel-norelabel)
+return 0;
+
+if (disk_seclabel disk_seclabel-label) {
+if (virParseOwnershipIds(disk_seclabel-label, user, group) 0)
+return -1;
+} else {
+if (virSecurityDACGetImageIds(secdef, priv, user, group))
+return -1;
+}
return virSecurityDACSetOwnership(path, user, group);
}
@@ -323,6 +335,9 @@ virSecurityDACSetSecurityImageLabel(virSecurityManagerPtr
mgr,
secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+if (secdef secdef-norelabel)
+return 0;
+
cbdata.manager = mgr;
cbdata.secdef = secdef;
return virDomainDiskDefForeachPath(disk,
@@ -339,6 +354,8 @@
virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
int migrated)
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virSecurityLabelDefPtr secdef;
+virSecurityDeviceLabelDefPtr disk_seclabel;
const char *src = virDomainDiskGetSource(disk);
if (!priv-dynamicOwnership)
@@ -347,6 +364,17 @@
virSecurityDACRestoreSecurityImageLabelInt(virSecurityManagerPtr mgr,
if (virDomainDiskGetType(disk) == VIR_STORAGE_TYPE_NETWORK)
return 0;
+secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+
+if (secdef secdef-norelabel)
+return 0;
+
+disk_seclabel = virDomainDiskDefGetSecurityLabelDef(disk,
+SECURITY_DAC_NAME);
+
+if (disk_seclabel disk_seclabel-norelabel)
+return 0;
+
/* Don't restore labels on readoly/shared disks, because
* other VMs may still be accessing these
* Alternatively we could iterate over all running
@@ -454,6 +482,9 @@ virSecurityDACSetSecurityHostdevLabel(virSecurityManagerPtr
mgr,
cbdata.manager = mgr;
cbdata.secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+if (cbdata.secdef cbdata.secdef-norelabel)
+return 0;
+
switch ((enum virDomainHostdevSubsysType) dev-source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: {
virUSBDevicePtr usb;
@@ -564,15 +595,18 @@ virSecurityDACRestoreSecuritySCSILabel(virSCSIDevicePtr
dev ATTRIBUTE_UNUSED,
static int
virSecurityDACRestoreSecurityHostdevLabel(virSecurityManagerPtr mgr,
- virDomainDefPtr def ATTRIBUTE_UNUSED,
+ virDomainDefPtr def,
virDomainHostdevDefPtr dev,
const char *vroot)
{
virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+virSecurityLabelDefPtr secdef;
int ret = -1;
-if (!priv-dynamicOwnership)
+secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_DAC_NAME);
+
+if (!priv-dynamicOwnership || (secdef secdef-norelabel))
return 0;
if (dev-mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS)
@@ -674,6 +708,9 @@
Re: [libvirt] [PATCH v2] libxl: Implement basic video device selection
On Fri, Apr 04, 2014 at 11:34:17AM +0100, Ian Campbell wrote:
On Fri, 2014-04-04 at 12:31 +0200, Stefan Bader wrote:
On 04.04.2014 11:48, Ian Campbell wrote:
On Fri, 2014-04-04 at 11:36 +0200, Stefan Bader wrote:
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if less
+ * is specified.
Build failures? Do you mean domain build rather than libvirt build?
I'm not sure about this fixing up the GIGO from the user side, but
that's a libvirt policy decision I suppose? If it were me I would just
let libxl provide the error and expect people to fix their domain config
rather than silently giving them something other than what they asked
for. What if increasing the VRAM causes a cascading failure due e.g. to
lack of memory? That's going to be tricky to debug I think!
In the end its a start a domain with such a config. Which seems to be what I
would end up with in my testing with an admittedly older version of
virt-manager
connecting to a libvirtd running an initial version of this patch without
that part.
The error seen on the front-end was something along the lines of failed to
get
enough memory to start the guest (the libxl log on the other side had the
better error message). And the gui always reduces the memory below the
minimum
for both the options (VGA and Xen).
That is the reason I went for meh, go for the minimum anyways.
Does the libvirt protocol require the client to provide all the sizes
etc with no provision for asking the server side to pick a sane default?
The XML does not have to include the VGA ram size. If it is omitted the
we fill in a default value after initial parsing is done. That defualt
can be hypervisor specific
And btw, it is already confusing enough as with cirrus, I get a 9M by
default
which is passed on to qemu on the command line and then ignored by it and
one
gets 32M in any way...
Yeah, we were silently ignoring ram size for cirrus since QEMU doesn't let
it be configured. Only QXL and I think stdvga is configurable.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] conf: track more fields in backing chain metadata
On 04/04/2014 03:31 AM, Peter Krempa wrote:
struct _virStorageFileMetadata {
-char *backingStore; /* Canonical name (absolute file, or protocol) */
-char *backingStoreRaw; /* If file, original name, possibly relative */
-char *directory; /* The directory containing basename of
backingStoreRaw */
-int backingStoreFormat; /* enum virStorageFileFormat */
-bool backingStoreIsFile;
+/* Name of this file as spelled by the user (top level) or
+ * metadata of the parent (if this is a backing store). */
+char *path;
+/* Canonical name of this file, used to detect loops in the
+ * backing store chain. */
+char *canonName;
+/* Directory to start from if backingStoreRaw is a relative file
+ * name */
+char *relDir;
+/* Name of the backing store recorded in metadata of the parent */
+char *backingStoreRaw;
Hmm, this field seems pretty redundant to me, IIUC it's the same data as
in path.
No, it's not.
Given the chain:
base - top
my goal is to have:
{ .path = top,
.canonName = /path/to/top,
.relDir = .,
.backingStoreRaw = base,
.backingMeta = {
.path = base,
.canonName = /path/to/base,
.relDir = .,
.backingStoreRaw = NULL,
.backingMeta = NULL,
}
}
OTOH, the path field should contain the canonical path as the target
is to convert it to the new storage file struct. In that case the
canonName will be redundant and backingStoreRaw needs to be kept to
track the original name.
Rather, path should be (but is not yet) the name as specified by the
user or the metadata of the parent file, unmodified (for both local
files and for network elements like gluster://server/vol/img); while
canonName should be the possibly munged name (munged to canonical
absolute name for local files, unmunged for network elements). Loop
detection is done by registering canonName in the hash table while
following the backing chain.
In any case ... one of them seems to be duplicate.
No, path is about the current element, while backingStoreRaw is about
the child element.
My other goal in this refactor is to make detection of missing backing
chains much saner. Right now, for the chain 'missing - foo', we have
this for 'foo':
{ .backingStoreRaw = missing,
.backingStore = NULL,
.backingStoreIsFile = false,
.backingMeta = NULL,
}
while for the chain 'gluster://.../base - foo', we have
{ .backingStoreRaw = NULL,
.backingStore = gluster://.../base,
.backingStoreIsFile = false,
.backingMeta = NULL,
}
where code that doesn't care about whether foo exists (such as storage
volume dumpxml) vs. code that DOES care (sVirt labeling before starting
a qemu domain) has to pay attention to multiple fields in the parent to
decide whether to raise an error; furthermore, since there is never any
chain emitted for a non-file backing, we can't support any network file
of anything other than raw. After the conversion, I envision:
{ .path = foo,
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
.backingStoreRaw = missing,
.backingMeta = NULL,
}
or maybe:
{ .path = foo,
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
.backingStoreRaw = missing,
.backingMeta = {
.path = missing,
.type = VIR_STORAGE_TYPE_NONE,
.format = VIR_STORAGE_FILE_NONE,
}
}
as the indication of a missing backing file, and:
{ .path = foo,
.type = VIR_STORAGE_TYPE_FILE,
.format = VIR_STORAGE_FILE_QCOW2,
.backingStoreRaw = gluster://.../base,
.backingMeta = {
.path = gluster://.../base,
.type = VIR_STORAGE_TYPE_NETWORK,
.format = VIR_STORAGE_FILE_RAW,
.backingStoreRaw = NULL,
.backingMeta = NULL,
}
}
as indication of a chain involving a network backing file, and where we
have a much easier task of identifying a network type resource that can
be something other than raw, so that we can actually have a chain of
multiple network devices if we know how to probe that network file (the
way we do for gluster).
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libxl: Implement basic video device selection
On Fri, 2014-04-04 at 14:51 +0200, Daniel P. Berrange wrote:
On Fri, Apr 04, 2014 at 11:34:17AM +0100, Ian Campbell wrote:
On Fri, 2014-04-04 at 12:31 +0200, Stefan Bader wrote:
On 04.04.2014 11:48, Ian Campbell wrote:
On Fri, 2014-04-04 at 11:36 +0200, Stefan Bader wrote:
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if
less
+ * is specified.
Build failures? Do you mean domain build rather than libvirt build?
I'm not sure about this fixing up the GIGO from the user side, but
that's a libvirt policy decision I suppose? If it were me I would just
let libxl provide the error and expect people to fix their domain config
rather than silently giving them something other than what they asked
for. What if increasing the VRAM causes a cascading failure due e.g. to
lack of memory? That's going to be tricky to debug I think!
In the end its a start a domain with such a config. Which seems to be
what I
would end up with in my testing with an admittedly older version of
virt-manager
connecting to a libvirtd running an initial version of this patch without
that part.
The error seen on the front-end was something along the lines of failed
to get
enough memory to start the guest (the libxl log on the other side had the
better error message). And the gui always reduces the memory below the
minimum
for both the options (VGA and Xen).
That is the reason I went for meh, go for the minimum anyways.
Does the libvirt protocol require the client to provide all the sizes
etc with no provision for asking the server side to pick a sane default?
The XML does not have to include the VGA ram size. If it is omitted the
we fill in a default value after initial parsing is done.
I guess the issue is that whatever client Stefan is using is including
the VGA ram size, with a value which it turns out is not allowed.
That defualt can be hypervisor specific
Great!
Ian.
And btw, it is already confusing enough as with cirrus, I get a 9M by
default
which is passed on to qemu on the command line and then ignored by it and
one
gets 32M in any way...
Yeah, we were silently ignoring ram size for cirrus since QEMU doesn't let
it be configured. Only QXL and I think stdvga is configurable.
Regards,
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libxl: Implement basic video device selection
On Fri, Apr 04, 2014 at 01:56:09PM +0100, Ian Campbell wrote:
On Fri, 2014-04-04 at 14:51 +0200, Daniel P. Berrange wrote:
On Fri, Apr 04, 2014 at 11:34:17AM +0100, Ian Campbell wrote:
On Fri, 2014-04-04 at 12:31 +0200, Stefan Bader wrote:
On 04.04.2014 11:48, Ian Campbell wrote:
On Fri, 2014-04-04 at 11:36 +0200, Stefan Bader wrote:
+/*
+ * Take the first defined video device (graphics card) to
display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind =
LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when
using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL
or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if
less
+ * is specified.
Build failures? Do you mean domain build rather than libvirt
build?
I'm not sure about this fixing up the GIGO from the user side, but
that's a libvirt policy decision I suppose? If it were me I would just
let libxl provide the error and expect people to fix their domain
config
rather than silently giving them something other than what they asked
for. What if increasing the VRAM causes a cascading failure due e.g.
to
lack of memory? That's going to be tricky to debug I think!
In the end its a start a domain with such a config. Which seems to be
what I
would end up with in my testing with an admittedly older version of
virt-manager
connecting to a libvirtd running an initial version of this patch
without that part.
The error seen on the front-end was something along the lines of
failed to get
enough memory to start the guest (the libxl log on the other side had
the
better error message). And the gui always reduces the memory below the
minimum
for both the options (VGA and Xen).
That is the reason I went for meh, go for the minimum anyways.
Does the libvirt protocol require the client to provide all the sizes
etc with no provision for asking the server side to pick a sane default?
The XML does not have to include the VGA ram size. If it is omitted the
we fill in a default value after initial parsing is done.
I guess the issue is that whatever client Stefan is using is including
the VGA ram size, with a value which it turns out is not allowed.
Yeah, that's probably a historical accident in virt-manager trying to guess
the default values itself rather than delegating.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/7] virsh-domain: Adapt to new error reporting
On 04.04.2014 11:12, Ján Tomko wrote:
On 04/02/2014 02:43 PM, Michal Privoznik wrote:
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/vcpupin| 1 -
tools/virsh-domain.c | 75 +---
2 files changed, 24 insertions(+), 52 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index a616216..638f62b 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -35,7 +35,6 @@ $abs_top_builddir/tools/virsh --connect test:///default vcpupin
test a 0,1 out
test $? = 1 || fail=1
cat \EOF exp || fail=1
error: Unable to parse integer parameter to --vcpu
-error: vcpupin: Invalid or missing vCPU number.
EOF
compare exp out || fail=1
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 73414f8..6e1eb17 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1120,7 +1120,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
if ((rv = vshCommandOptULongLong(cmd, total-bytes-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
@@ -1129,7 +1129,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, read-bytes-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
@@ -1138,7 +1138,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, write-bytes-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
@@ -1147,7 +1147,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, total-iops-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
@@ -1156,7 +1156,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, read-iops-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
@@ -1165,7 +1165,7 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
}
if ((rv = vshCommandOptULongLong(cmd, write-iops-sec, value)) 0) {
-goto interror;
+goto error;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_WRITE_IOPS_SEC,
@@ -1216,10 +1216,6 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
error:
vshError(ctl, %s, _(Unable to change block I/O throttle));
goto cleanup;
-
- interror:
-vshError(ctl, %s, _(Unable to parse integer parameter));
-goto cleanup;
}
These should jump to cleanup, not error.
Oh right, I've misread the label under interror. Since you're requesting
the function signature change in 1/7 I'll post v2. However, this time
it's going to be a single patch.
Michal
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libxl: Implement basic video device selection
On 04.04.2014 14:56, Ian Campbell wrote:
On Fri, 2014-04-04 at 14:51 +0200, Daniel P. Berrange wrote:
On Fri, Apr 04, 2014 at 11:34:17AM +0100, Ian Campbell wrote:
On Fri, 2014-04-04 at 12:31 +0200, Stefan Bader wrote:
On 04.04.2014 11:48, Ian Campbell wrote:
On Fri, 2014-04-04 at 11:36 +0200, Stefan Bader wrote:
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if less
+ * is specified.
Build failures? Do you mean domain build rather than libvirt build?
I'm not sure about this fixing up the GIGO from the user side, but
that's a libvirt policy decision I suppose? If it were me I would just
let libxl provide the error and expect people to fix their domain config
rather than silently giving them something other than what they asked
for. What if increasing the VRAM causes a cascading failure due e.g. to
lack of memory? That's going to be tricky to debug I think!
In the end its a start a domain with such a config. Which seems to be what
I
would end up with in my testing with an admittedly older version of
virt-manager
connecting to a libvirtd running an initial version of this patch without
that part.
The error seen on the front-end was something along the lines of failed
to get
enough memory to start the guest (the libxl log on the other side had the
better error message). And the gui always reduces the memory below the
minimum
for both the options (VGA and Xen).
That is the reason I went for meh, go for the minimum anyways.
Does the libvirt protocol require the client to provide all the sizes
etc with no provision for asking the server side to pick a sane default?
The XML does not have to include the VGA ram size. If it is omitted the
we fill in a default value after initial parsing is done.
I guess the issue is that whatever client Stefan is using is including
the VGA ram size, with a value which it turns out is not allowed.
Right and the current fixup code is in there because I am too lazy to be
bothered to use virsh to fix up the vram size all the times. And in some way I
expected other users to be of the same mind. Or even not to find out what went
wrong at all.
I am open to let this drop on the upstream side and only carry the delta
locally. Whichever sounds more suitable for the upstream maintainers.
-Stefan
That defualt can be hypervisor specific
Great!
Ian.
And btw, it is already confusing enough as with cirrus, I get a 9M by
default
which is passed on to qemu on the command line and then ignored by it and
one
gets 32M in any way...
Yeah, we were silently ignoring ram size for cirrus since QEMU doesn't let
it be configured. Only QXL and I think stdvga is configurable.
Regards,
Daniel
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] tests: build viridentitytest only WITH_ATTR.
Commit d7c4e0036 assumed all SELinux tests depended upon securityselinuxhelper need xattr support, but forgot to move viridentitytest under WITH_ATTR. Reported-by: Nehal J Wani nehaljw.k...@gmail.com Signed-off-by: Jincheng Miao jm...@redhat.com --- tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 6e15af8..7a15295 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -146,7 +146,6 @@ test_programs = virshtest sockettest \ virpcitest \ virendiantest \ virfiletest \ - viridentitytest \ viriscsitest \ virkeycodetest \ virlockspacetest \ @@ -188,7 +187,8 @@ endif WITH_DBUS if WITH_SECDRIVER_SELINUX if WITH_ATTR -test_programs += securityselinuxtest +test_programs += securityselinuxtest \ + viridentitytest if WITH_QEMU test_programs += securityselinuxlabeltest endif WITH_QEMU -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Enhancing clean-traffic to work with IPv6
On 4/4/2014 4:55 AM, Daniel P. Berrange wrote: On Thu, Apr 03, 2014 at 05:28:35PM -0400, Brian Rak wrote: I'm looking into adding IPv6 support to the nwfilter clean-traffic rules, but I'm unsure of the best approach to this. I'm planning on sending patches once I get this correct, so I'm trying to figure out what way fits in best. There's a couple different ways I can think of: 1) Explicitly add v6 rules to the existing clean-traffic rules. This would enable IPv6 for guests whenever libvirt was upgraded, which may be a problem. 2) Add another filter chain (clean-ipv6-traffic) that would do the same thing as clean-traffic, just for IPv6 3) Add another filter chain (clean-ipv6-ipv4-traffic), that would clean IPv6 traffic, and include the clean-traffic filter set The limitation here is that IP learning will not work for IPv6, so actually using IPv6 is going to require passing in parameters to filter specifying what ranges the guest should be allowed to use. I think this rules out #1. Why do you say IP learning won't work ? The current impl of IP learning only supports IPv4, but AFAIK, it should be viable to enhance it to detect an address from the first outbound IPv6 packet, or by snooping DHCPv6 responses, just as we do for IPv4 Regards, Daniel Right, that was mainly my point. Currently, IP learning does not support IPv6.It's probably possible to add support for this, but since we don't actually make use of IP learning at this point, it's not something I was planning on implementing. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v2] libxl: Implement basic video device selection
On 04.04.2014 15:17, Daniel P. Berrange wrote:
On Fri, Apr 04, 2014 at 11:36:39AM +0200, Stefan Bader wrote:
+static int
+libxlSetBuildGraphics(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+libxl_domain_build_info *b_info = d_config-b_info;
+
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if less
+ * is specified.
+ */
+switch (b_info-device_model_version) {
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+min_vram = 8 * 1024;
+break;
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+default:
+min_vram = 16 * 1024;
+}
+break;
+default:
+/*
+ * Ignore any other device type and use Cirrus. Again fix
+ * up the minimal VRAM to what libxl expects.
+ */
We shouldn't do that 'default'. For any device type that Xen can't support
we should report VIR_ERR_CONFIG_UNSUPPORTED.
Ok, I could remove that. At some point it might be possible to enhance that
again. But that requires more careful thinking. At least with
device_model_version QEMU_XEN (not QEMU_XEN_TRADITIONAL and not to confuse with
device_model which just sets the path ;)) at least in theory there should be the
same support as for kvm... Anyway, just loud thinking.
Would you also rather want the VRAM fixup to be removed as well? I probably wait
a bit more for other feedback and then would do a v3...
-Stefan
Regards,
Daniel
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] QoS: make tc filters match all traffic
Up until now the traffic control filters for the vNIC QoS were matching only ip traffic. For egress traffic that was unnoticed because the unmatched traffic would just go to the default htb class and be shaped anyway. For ingress, though, since the policing of the rate is done by the filter itself. The problem is solved by changing protocol to all and making anything match the filter. Bug-Url: https://bugzilla.redhat.com/108 Signed-off-by: Antoni S. Puimedon asegu...@redhat.com --- src/util/virnetdevbandwidth.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index b06ed4c..1e00116 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -193,7 +193,7 @@ virNetDevBandwidthSet(const char *ifname, virCommandFree(cmd); cmd = virCommandNew(TC); virCommandAddArgList(cmd, filter, add, dev, ifname, parent, - 1:0, protocol, ip, handle, 1, fw, + 1:0, protocol, all, handle, 1, fw, flowid, 1, NULL); if (virCommandRun(cmd, NULL) 0) @@ -221,9 +221,10 @@ virNetDevBandwidthSet(const char *ifname, virCommandFree(cmd); cmd = virCommandNew(TC); +/* Set filter to match all ingress traffic */ virCommandAddArgList(cmd, filter, add, dev, ifname, parent, - :, protocol, ip, u32, match, ip, - src, 0.0.0.0/0, police, rate, average, + :, protocol, all, u32, match, u32, + 0, 0, police, rate, average, burst, burst, mtu, 64kb, drop, flowid, :1, NULL); -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv2] virsh: Make vshCommandOpt* report error
Currently, the virsh code is plenty of the following pattern:
if (vshCommandOptUInt(..) 0) {
vshError(...);
goto cleanup;
}
It doesn't make much sense to repeat the code everywhere.
Moreover, some functions from the family already report
error some of them don't.
Signed-off-by: Michal Privoznik mpriv...@redhat.com
---
tests/vcpupin| 2 +-
tools/virsh-domain-monitor.c | 7 +--
tools/virsh-domain.c | 102 +++
tools/virsh-host.c | 25 +++
tools/virsh-interface.c | 4 +-
tools/virsh-network.c| 4 +-
tools/virsh-volume.c | 16 ++-
tools/virsh.c| 99 +++--
tools/virsh.h| 24 +-
9 files changed, 140 insertions(+), 143 deletions(-)
diff --git a/tests/vcpupin b/tests/vcpupin
index f1fb038..638f62b 100755
--- a/tests/vcpupin
+++ b/tests/vcpupin
@@ -34,7 +34,7 @@ fail=0
$abs_top_builddir/tools/virsh --connect test:///default vcpupin test a 0,1
out 21
test $? = 1 || fail=1
cat \EOF exp || fail=1
-error: vcpupin: Invalid or missing vCPU number.
+error: Unable to parse integer parameter to --vcpu
EOF
compare exp out || fail=1
diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c
index 18d551a..0da4f91 100644
--- a/tools/virsh-domain-monitor.c
+++ b/tools/virsh-domain-monitor.c
@@ -324,12 +324,9 @@ cmdDomMemStat(vshControl *ctl, const vshCmd *cmd)
/* Providing a period will adjust the balloon driver collection period.
* This is not really an unsigned long, but it
*/
-if ((rv = vshCommandOptInt(cmd, period, period)) 0) {
-vshError(ctl, %s,
- _(Unable to parse integer parameter.));
+if ((rv = vshCommandOptInt(ctl, cmd, period, period)) 0) {
goto cleanup;
-}
-if (rv 0) {
+} else if (rv 0) {
if (period 0) {
vshError(ctl, _(Invalid collection period value '%d'), period);
goto cleanup;
diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c
index 73414f8..821ed62 100644
--- a/tools/virsh-domain.c
+++ b/tools/virsh-domain.c
@@ -1119,8 +1119,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptStringReq(ctl, cmd, device, disk) 0)
goto cleanup;
-if ((rv = vshCommandOptULongLong(cmd, total-bytes-sec, value)) 0) {
-goto interror;
+if ((rv = vshCommandOptULongLong(ctl, cmd, total-bytes-sec, value))
0) {
+goto cleanup;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC,
@@ -1128,8 +1128,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
goto save_error;
}
-if ((rv = vshCommandOptULongLong(cmd, read-bytes-sec, value)) 0) {
-goto interror;
+if ((rv = vshCommandOptULongLong(ctl, cmd, read-bytes-sec, value)) 0)
{
+goto cleanup;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_READ_BYTES_SEC,
@@ -1137,8 +1137,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
goto save_error;
}
-if ((rv = vshCommandOptULongLong(cmd, write-bytes-sec, value)) 0) {
-goto interror;
+if ((rv = vshCommandOptULongLong(ctl, cmd, write-bytes-sec, value))
0) {
+goto cleanup;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_WRITE_BYTES_SEC,
@@ -1146,8 +1146,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
goto save_error;
}
-if ((rv = vshCommandOptULongLong(cmd, total-iops-sec, value)) 0) {
-goto interror;
+if ((rv = vshCommandOptULongLong(ctl, cmd, total-iops-sec, value)) 0)
{
+goto cleanup;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_IOPS_SEC,
@@ -1155,8 +1155,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
goto save_error;
}
-if ((rv = vshCommandOptULongLong(cmd, read-iops-sec, value)) 0) {
-goto interror;
+if ((rv = vshCommandOptULongLong(ctl, cmd, read-iops-sec, value)) 0) {
+goto cleanup;
} else if (rv 0) {
if (virTypedParamsAddULLong(params, nparams, maxparams,
VIR_DOMAIN_BLOCK_IOTUNE_READ_IOPS_SEC,
@@ -1164,8 +1164,8 @@ cmdBlkdeviotune(vshControl *ctl, const vshCmd *cmd)
goto save_error;
}
-if ((rv = vshCommandOptULongLong(cmd, write-iops-sec, value)) 0) {
-goto interror;
+if ((rv = vshCommandOptULongLong(ctl, cmd, write-iops-sec, value)) 0)
{
+goto cleanup;
} else if (rv 0) {
if
Re: [libvirt] [PATCH v2] libxl: Implement basic video device selection
On Fri, Apr 04, 2014 at 03:36:38PM +0200, Stefan Bader wrote:
On 04.04.2014 15:17, Daniel P. Berrange wrote:
On Fri, Apr 04, 2014 at 11:36:39AM +0200, Stefan Bader wrote:
+static int
+libxlSetBuildGraphics(virDomainDefPtr def, libxl_domain_config *d_config)
+{
+libxl_domain_build_info *b_info = d_config-b_info;
+
+/*
+ * Take the first defined video device (graphics card) to display
+ * on the first graphics device (display).
+ * Right now only type and vram info is used and anything beside
+ * type xen and vga is mapped to cirrus.
+ */
+if (def-nvideos) {
+unsigned int min_vram = 8 * 1024;
+
+switch (def-videos[0]-type) {
+case VIR_DOMAIN_VIDEO_TYPE_VGA:
+case VIR_DOMAIN_VIDEO_TYPE_XEN:
+b_info-u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
+/*
+ * Libxl enforces a minimal VRAM size of 8M when using
+ * LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL or
+ * 16M for LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN.
+ * Avoid build failures and go with the minimum if less
+ * is specified.
+ */
+switch (b_info-device_model_version) {
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+min_vram = 8 * 1024;
+break;
+case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+default:
+min_vram = 16 * 1024;
+}
+break;
+default:
+/*
+ * Ignore any other device type and use Cirrus. Again fix
+ * up the minimal VRAM to what libxl expects.
+ */
We shouldn't do that 'default'. For any device type that Xen can't support
we should report VIR_ERR_CONFIG_UNSUPPORTED.
Ok, I could remove that. At some point it might be possible to enhance that
again. But that requires more careful thinking. At least with
device_model_version QEMU_XEN (not QEMU_XEN_TRADITIONAL and not to confuse
with
device_model which just sets the path ;)) at least in theory there should be
the
same support as for kvm... Anyway, just loud thinking.
Would you also rather want the VRAM fixup to be removed as well? I probably
wait
a bit more for other feedback and then would do a v3...
I'd suggest, do a first patch which removes the 'default:' case and sets an
error and honours the VRAM size. Then do a second patch which adds the VRAM
workaround, so we can discuss it separately from the main enablement.
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cpu: Properly check input parameters
On 03/26/14 16:14, Jiri Denemark wrote: Most of the APIs in CPU driver do not expect to get NULL for input parameters. Let's mark them with ATTRIBUTE_NONNULL and also check for some members of virCPUDef when the APIs expect them have some specific values. Signed-off-by: Jiri Denemark jdene...@redhat.com --- src/cpu/cpu.c | 48 +--- src/cpu/cpu.h | 36 2 files changed, 61 insertions(+), 23 deletions(-) ACK, Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Enhancing clean-traffic to work with IPv6
On Fri, Apr 04, 2014 at 09:35:26AM -0400, Brian Rak wrote: On 4/4/2014 4:55 AM, Daniel P. Berrange wrote: On Thu, Apr 03, 2014 at 05:28:35PM -0400, Brian Rak wrote: I'm looking into adding IPv6 support to the nwfilter clean-traffic rules, but I'm unsure of the best approach to this. I'm planning on sending patches once I get this correct, so I'm trying to figure out what way fits in best. There's a couple different ways I can think of: 1) Explicitly add v6 rules to the existing clean-traffic rules. This would enable IPv6 for guests whenever libvirt was upgraded, which may be a problem. 2) Add another filter chain (clean-ipv6-traffic) that would do the same thing as clean-traffic, just for IPv6 3) Add another filter chain (clean-ipv6-ipv4-traffic), that would clean IPv6 traffic, and include the clean-traffic filter set The limitation here is that IP learning will not work for IPv6, so actually using IPv6 is going to require passing in parameters to filter specifying what ranges the guest should be allowed to use. I think this rules out #1. Why do you say IP learning won't work ? The current impl of IP learning only supports IPv4, but AFAIK, it should be viable to enhance it to detect an address from the first outbound IPv6 packet, or by snooping DHCPv6 responses, just as we do for IPv4 Right, that was mainly my point. Currently, IP learning does not support IPv6.It's probably possible to add support for this, but since we don't actually make use of IP learning at this point, it's not something I was planning on implementing. Ok, but from the POV of the default out-of-the-box 'clean-traffic' filter that we ship, I think that relying on IP learning is the best behaviour. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add V6LOCAL parameter to nwfilter rules.
On 4/4/2014 4:48 AM, Daniel P. Berrange wrote:
On Wed, Apr 02, 2014 at 03:40:14PM -0400, Brian Rak wrote:
@@ -163,6 +164,28 @@ virNWFilterVarHashmapAddStdValues(virNWFilterHashTablePtr
table,
%s, _(Could not add variable 'MAC' to
hashmap));
return -1;
}
+
+virMacAddr parsedMac;
+if (virMacAddrParse(macaddr, parsedMac) == 0)
+{
+parsedMac.addr[0] ^= 2;
+
+char euiMacAddr[26];
+snprintf(euiMacAddr, sizeof(euiMacAddr),
fe80::%x%x:%xff:fe%x:%x%x, parsedMac.addr[0], parsedMac.addr[1],
parsedMac.addr[2],
+parsedMac.addr[3], parsedMac.addr[4], parsedMac.addr[5]);
Unfortunately this IPv6 addressing scheme isn't portable. Windows Vista
will create link local addresses which are completely random, not based
on the MAC address.
http://technet.microsoft.com/en-us/magazine/2007.08.cableguy.aspx
I wonder if there's a way todo something more clever for IPv6 to learn
the addresses, we as do for IPv4 address learning, or snoop route
advertisment traffic as we do for DHCP
Regards,
Daniel
Vista can be configured to use the EUI64 format though (as per that
link). I don't think that we can really trust that the guest is not
malicious, so I'm not sure that trying to learn the link-local IPv6
address would be secure.
I'm not sure if there's other security issues or not, but a malicious
guest using another guest's link local address would definitely cause
some problems.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add V6LOCAL parameter to nwfilter rules.
On Fri, Apr 04, 2014 at 10:08:26AM -0400, Brian Rak wrote:
On 4/4/2014 4:48 AM, Daniel P. Berrange wrote:
On Wed, Apr 02, 2014 at 03:40:14PM -0400, Brian Rak wrote:
@@ -163,6 +164,28 @@
virNWFilterVarHashmapAddStdValues(virNWFilterHashTablePtr table,
%s, _(Could not add variable 'MAC' to
hashmap));
return -1;
}
+
+virMacAddr parsedMac;
+if (virMacAddrParse(macaddr, parsedMac) == 0)
+{
+parsedMac.addr[0] ^= 2;
+
+char euiMacAddr[26];
+snprintf(euiMacAddr, sizeof(euiMacAddr),
fe80::%x%x:%xff:fe%x:%x%x, parsedMac.addr[0], parsedMac.addr[1],
parsedMac.addr[2],
+parsedMac.addr[3], parsedMac.addr[4], parsedMac.addr[5]);
Unfortunately this IPv6 addressing scheme isn't portable. Windows Vista
will create link local addresses which are completely random, not based
on the MAC address.
http://technet.microsoft.com/en-us/magazine/2007.08.cableguy.aspx
I wonder if there's a way todo something more clever for IPv6 to learn
the addresses, we as do for IPv4 address learning, or snoop route
advertisment traffic as we do for DHCP
Vista can be configured to use the EUI64 format though (as per that
link). I don't think that we can really trust that the guest is not
malicious, so I'm not sure that trying to learn the link-local IPv6
address would be secure.
I'm not sure if there's other security issues or not, but a
malicious guest using another guest's link local address would
definitely cause some problems.
NB you have to decide what the threat scenario is. The learning code is
obviously vulnerable if your threat is a guest that is malicious from
the time it is first booted we weren't attempting to address that
scenario though. The learning code is intended to protect against the
more limited scenario where your guest is initially trusted, but gets
compromised while running. This is good enough for an out of the box
config.
If you care about stronger security then you only want to rely on learning
from a trusted DHCP server response, or use a hardcoded IP address variable
in the guest XML config. These cease to be zero-config though since the
host admin must set the IP addr of the trusted DHCP server, or set the
guest IP addr(s).
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] NFS storage pool: Fix libvirtd crash due to refactor edit
Commit id '18642d10' refactored the call to virCommandRunRegex() inside a new function virStorageBackendFileSystemNetFindNFSPoolSources(), but the cut-n-paste didn't remove the state. Now that state is passed by reference, it results in a libvirtd core with a messages entry: ...internal error: unknown storage pool type Unknow Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 1d85871..4e4a7ae 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -266,7 +266,7 @@ virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) if (virCommandRunRegex(cmd, 1, regexes, vars, virStorageBackendFileSystemNetFindPoolSourcesFunc, - state, NULL) 0) + state, NULL) 0) virResetLastError(); virCommandFree(cmd); -- 1.8.5.3 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5] conf: track more fields in backing chain metadata
On 04/04/14 14:54, Eric Blake wrote:
On 04/04/2014 03:31 AM, Peter Krempa wrote:
struct _virStorageFileMetadata {
-char *backingStore; /* Canonical name (absolute file, or protocol) */
-char *backingStoreRaw; /* If file, original name, possibly relative */
-char *directory; /* The directory containing basename of
backingStoreRaw */
-int backingStoreFormat; /* enum virStorageFileFormat */
-bool backingStoreIsFile;
+/* Name of this file as spelled by the user (top level) or
+ * metadata of the parent (if this is a backing store). */
+char *path;
+/* Canonical name of this file, used to detect loops in the
+ * backing store chain. */
+char *canonName;
+/* Directory to start from if backingStoreRaw is a relative file
+ * name */
+char *relDir;
+/* Name of the backing store recorded in metadata of the parent */
Maybe then change parent to this image to un-confuse me :)
+char *backingStoreRaw;
Hmm, this field seems pretty redundant to me, IIUC it's the same data as
in path.
No, it's not.
Given the chain:
base - top
my goal is to have:
{ .path = top,
.canonName = /path/to/top,
.relDir = .,
.backingStoreRaw = base,
.backingMeta = {
.path = base,
.canonName = /path/to/base,
.relDir = .,
.backingStoreRaw = NULL,
.backingMeta = NULL,
}
}
my mind was poisoned with the current way the code is filling the fields
and a little bit with the comment.
ACK the refactor makes sense. Maybe it's worth changing the wording a
bit though.
Peter
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] [PATCH] spapr: skip adding usb keyboard/mouse in case of -nodefaults
[adding libvir-list] On 04/04/2014 05:23 AM, Markus Armbruster wrote: Paolo Bonzini pbonz...@redhat.com writes: Il 04/04/2014 12:58, Markus Armbruster ha scritto: Have you considered extending QEMUMachineInitArgs instead of adding this function? Did not think of this option earlier. You mean doing something like this? Yes. Looks nicer, doesn't it? I still think it's a libvirt bug. Mixing -nodefaults and -usb and -device is looking for trouble I think. -usb is a do-what-I-mean kind of option and it makes sense for it to add a keyboard and mouse, even with -nodefaults. I agree that management tools should not use -usb, except when they want to manage ancient versions of QEMU for some reason. We've tried to make libvirt avoid -usb when it knows it is targetting a new enough qemu. But I'm not the one most familiar with that code. At this point, I'm just trying to facilitate discussions, and make sure the right people are looking at this - what versions of qemu vs. libvirt have you tested, and what does libvirt still need to patch to avoid -usb, and/or qemu patch so that libvirt can correctly probe that -usb is not needed? -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 2/3] qemu: RDMA migration support using 'rdma' URI
On 04/04/2014 12:19 AM, Michael R. Hines wrote: @@ -2561,6 +2570,10 @@ virQEMUCapsInitQMPMonitor(virQEMUCapsPtr qemuCaps, if (qemuCaps-version = 1006000) virQEMUCapsSet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY); +if (qemuCaps-version = 200) +virQEMUCapsSet(qemuCaps, QEMU_CAPS_MIGRATE_QEMU_RDMA); + + And here we need a better check for rdma migration. What if someone compiles QEMU without RDMA support? Better than hard-coding it to a version string is to probe the results of query-migrate-capabilities and only setting the capability if the resulting list includes rdma-pin-all, as that will serve as a reliable witness of qemu being new enough to support rdma without an x- prefix. These comments I don't understand: Why can't we depend on the version number here? Isn't that what it was designed for? No. Features get backported downstream all the time, to something that ostensibly fails the version number check. For example, RHEL 6 qemu claims to be 0.10, but has backported many features that are much closer to upstream qemu 1.7. Libvirt basing a feature check on a version number will guess wrong if RHEL 7 backports the upstream qemu 2.0 feature to whatever 1.x version of downstream qemu lives in RHEL. Whereas probing for the _feature_ (by calling query-migrate-capabilities and looking for rdma-pin-all) will work for ALL qemu builds, regardless of whether that qemu calls itself 2.0 or not. If someone compiles QEMU without RDMA support - why does libvirt need to know about that? Shouldn't the admin know what their hardware is capable of - otherwise, if they try to specify rdma://hostname as a migration option, they will get a failure - which would be the correct behavior - they tried to do something without verifying that their hardware was capable of handling it. Getting an error message from qemu about an unsupported option doesn't always read very well - having libvirt query qemu to see if the option is supported, so that libvirt controls the error message when it is not, often leads to a nicer user experience. Checking the capability list won't help here either: It will still be in the list even if we don't compile QEMU with RDMA support. And if someone sets the capability anyway, it will just get ignored by QEMU since RDMA support was not available at compile time. If rdma-pin-all appears in the query-migrate-capabilities output of a qemu binary compiled without RDMA support, that is a bug in qemu, and should be fixed, preferably before qemu 2.0 is out. The whole point of feature detection is to be a reliable way of learning whether the feature is present and supported. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] NFS storage pool: Fix libvirtd crash due to refactor edit
On 04/04/2014 08:36 AM, John Ferlan wrote: Commit id '18642d10' refactored the call to virCommandRunRegex() inside a new function virStorageBackendFileSystemNetFindNFSPoolSources(), but the cut-n-paste didn't remove the state. Now that state is passed by reference, it results in a libvirtd core with a messages entry: ...internal error: unknown storage pool type Unknow Signed-off-by: John Ferlan jfer...@redhat.com --- src/storage/storage_backend_fs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index 1d85871..4e4a7ae 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -266,7 +266,7 @@ virStorageBackendFileSystemNetFindNFSPoolSources(virNetfsDiscoverState *state) if (virCommandRunRegex(cmd, 1, regexes, vars, virStorageBackendFileSystemNetFindPoolSourcesFunc, - state, NULL) 0) + state, NULL) 0) Eww that the compiler couldn't flag this - but that really is the void* opaque argument where the burden is on us to pass the correct type. ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] PCI: Introduce new device binding path using pci_dev.driver_override
The driver_override field allows us to specify the driver for a device rather than relying on the driver to provide a positive match of the device. This shortcuts the existing process of looking up the vendor and device ID, adding them to the driver new_id, binding the device, then removing the ID, but it also provides a couple advantages. First, the above existing process allows the driver to bind to any device matching the new_id for the window where it's enabled. This is often not desired, such as the case of trying to bind a single device to a meta driver like pci-stub or vfio-pci. Using driver_override we can do this deterministically using: echo pci-stub /sys/bus/pci/devices/:03:00.0/driver_override echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Previously we could not invoke drivers_probe after adding a device to new_id for a driver as we get non-deterministic behavior whether the driver we intend or the standard driver will claim the device. Now it becomes a deterministic process, only the driver matching driver_override will probe the device. To return the device to the standard driver, we simply clear the driver_override and reprobe the device: echo /sys/bus/pci/devices/:03:00.0/preferred_driver echo :03:00.0 /sys/bus/pci/devices/:03:00.0/driver/unbind echo :03:00.0 /sys/bus/pci/drivers_probe Another advantage to this approach is that we can specify a driver override to force a specific binding or prevent any binding. For instance when an IOMMU group is exposed to userspace through VFIO we require that all devices within that group are owned by VFIO. However, devices can be hot-added into an IOMMU group, in which case we want to prevent the device from binding to any driver (preferred driver = none) or perhaps have it automatically bind to vfio-pci. With driver_override it's a simple matter for this field to be set internally when the device is first discovered to prevent driver matches. Signed-off-by: Alex Williamson alex.william...@redhat.com --- Changes since RFC: - Add ABI documentation (gregkh) - Documentation wording clarification (Christoffer) Nobody puked on the RFC and platform folks have posted a working version of this for platform devices, so I guess the only thing left to do is formally propose this as a new driver binding mechanism. I don't see much incentive to push this into the driver core since the match ultimately needs to be done by the bus driver. I think this is therefore like new_id/remove_id where PCI and USB implement separate, but consistent interfaces. I've pruned the CC list a bit from the RFC, but I've added libvirt folks since I expect they would be the first userspace tool that would adopt this. Thanks, Alex Documentation/ABI/testing/sysfs-bus-pci | 21 drivers/pci/pci-driver.c| 25 +-- drivers/pci/pci-sysfs.c | 40 +++ include/linux/pci.h |1 + 4 files changed, 84 insertions(+), 3 deletions(-) diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci index a3c5a66..898ddc4 100644 --- a/Documentation/ABI/testing/sysfs-bus-pci +++ b/Documentation/ABI/testing/sysfs-bus-pci @@ -250,3 +250,24 @@ Description: valid. For example, writing a 2 to this file when sriov_numvfs is not 0 and not 2 already will return an error. Writing a 10 when the value of sriov_totalvfs is 8 will return an error. + +What: /sys/bus/pci/devices/.../driver_override +Date: April 2014 +Contact: Alex Williamson alex.william...@redhat.com +Description: + This file allows the driver for a device to be specified which + will override standard static and dynamic ID matching. When + specified, only a driver with a name matching the value written + to driver_override will have an opportunity to bind to the + device. The override is specified by writing a string to the + driver_override file (echo pci-stub driver_override) and + may be cleared with an empty string (echo driver_override). + This returns the device to standard matching rules binding. + Writing to driver_override does not automatically unbind the + device from its current driver or make any attempt to + automatically load the specified driver. If no driver with a + matching name is currently loaded in the kernel, the device + will not bind to any driver. This also allows devices to + opt-out of driver binding using a driver_override name such as + none. Only a single driver may be specified in the override, + there is no support for parsing delimiters. diff --git
Re: [libvirt] [RFC PATCH v2 3/3] qemu: memory pre-pinning support for RDMA migration
On 04/04/2014 12:29 AM, Michael R. Hines wrote: Yes, it's present, but it still does not guarantee that QEMU supports it if RDMA was compiled out - only the version number is a (minimal) guarantee, and even then the hardware can still throw an error if RDMA itself is not supported. Which sounds like you want a feature test (is it supported in the build you are talking to, answer is always correct) and not a version test (is the build you are talking to new enough to possibly have the feature, but if you guess wrong you may get an error from the hardware) I'm still not seeing what's wrong with depending on the version number since other features are also depending on the version number. Every feature where we have to guess based on version number is due to a bug in qemu for not providing enough information for libvirt to probe for the feature's existence. We hate those features, and I have been lobbying hard on the qemu list that all NEW features should be discoverable. rdma is one such feature - I recall you making changes in your series there so that it is discoverable in response to my early review comments - so now please USE that methodology from libvirt. Are you guys suggesting that we stop depending on version altogether for *all* QEMU features? Yes, that would be the ideal world. We won't get there any time soon, but feature checks are ALWAYS better than version checks. In fact, that was the philosophy that led to the creation of autoconf. Feature checks are inherently more portable to a wider range of backported forks than any database of version number checks could ever hope to be. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] hash: add common utility functions
I almost wrote a hash value free function that just called
VIR_FREE, then realized I couldn't be the first person to
do that. Sure enough, it was worth factoring into a common
helper routine.
Furthermore, in a few places we were passing raw free() as
the cleanup function; whereas going through VIR_FREE() ensures
that any (temporary) tracing or other memory stress testing
that we add to viralloc.c will not be bypassed.
* src/util/virhash.h (virHashValueFree): New function.
* src/util/virhash.c (virHashValueFree): Implement it.
* src/util/virobject.h (virObjectFreeHashData): New function.
* src/libvirt_private.syms (virhash.h, virobject.h): Export them.
* src/nwfilter/nwfilter_learnipaddr.c (virNWFilterLearnInit): Use
common function.
* src/qemu/qemu_capabilities.c (virQEMUCapsCacheNew): Likewise.
* src/qemu/qemu_command.c (qemuDomainCCWAddressSetCreate):
Likewise.
* src/qemu/qemu_monitor.c (qemuMonitorGetBlockInfo): Likewise.
* src/qemu/qemu_process.c (qemuProcessWaitForMonitor): Likewise.
* src/util/virclosecallbacks.c (virCloseCallbacksNew): Likewise.
* src/util/virkeyfile.c (virKeyFileParseGroup): Likewise.
* tests/qemumonitorjsontest.c
(testQemuMonitorJSONqemuMonitorJSONGetBlockInfo): Likewise.
Signed-off-by: Eric Blake ebl...@redhat.com
---
It turns out I may end up not using the common helper in my
code after all, but the cleanup is still worth posting.
src/libvirt_private.syms| 2 ++
src/nwfilter/nwfilter_learnipaddr.c | 9 +
src/qemu/qemu_capabilities.c| 9 +
src/qemu/qemu_command.c | 8 +---
src/qemu/qemu_monitor.c | 2 +-
src/qemu/qemu_process.c | 6 +-
src/util/virclosecallbacks.c| 11 ++-
src/util/virhash.c | 9 -
src/util/virhash.h | 5 -
src/util/virkeyfile.c | 9 ++---
src/util/virobject.c| 17 -
src/util/virobject.h| 3 ++-
tests/qemumonitorjsontest.c | 6 +++---
13 files changed, 44 insertions(+), 52 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 2d12105..6b68a43 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1290,6 +1290,7 @@ virHashSize;
virHashSteal;
virHashTableSize;
virHashUpdateEntry;
+virHashValueFree;
# util/virhook.h
@@ -1628,6 +1629,7 @@ virClassIsDerivedFrom;
virClassName;
virClassNew;
virObjectFreeCallback;
+virObjectFreeHashData;
virObjectIsClass;
virObjectLock;
virObjectLockableNew;
diff --git a/src/nwfilter/nwfilter_learnipaddr.c
b/src/nwfilter/nwfilter_learnipaddr.c
index 0eb5e7e..1ffed78 100644
--- a/src/nwfilter/nwfilter_learnipaddr.c
+++ b/src/nwfilter/nwfilter_learnipaddr.c
@@ -188,13 +188,6 @@ virNWFilterLockIface(const char *ifname)
}
-static void
-freeIfaceLock(void *payload, const void *name ATTRIBUTE_UNUSED)
-{
-VIR_FREE(payload);
-}
-
-
void
virNWFilterUnlockIface(const char *ifname)
{
@@ -818,7 +811,7 @@ virNWFilterLearnInit(void)
return -1;
}
-ifaceLockMap = virHashCreate(0, freeIfaceLock);
+ifaceLockMap = virHashCreate(0, virHashValueFree);
if (!ifaceLockMap) {
virNWFilterLearnShutdown();
return -1;
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index e1d04ff..2c8ec10 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -3288,13 +3288,6 @@ bool virQEMUCapsIsValid(virQEMUCapsPtr qemuCaps)
}
-static void
-virQEMUCapsHashDataFree(void *payload, const void *key ATTRIBUTE_UNUSED)
-{
-virObjectUnref(payload);
-}
-
-
virQEMUCapsCachePtr
virQEMUCapsCacheNew(const char *libDir,
const char *cacheDir,
@@ -3313,7 +3306,7 @@ virQEMUCapsCacheNew(const char *libDir,
return NULL;
}
-if (!(cache-binaries = virHashCreate(10, virQEMUCapsHashDataFree)))
+if (!(cache-binaries = virHashCreate(10, virObjectFreeHashData)))
goto error;
if (VIR_STRDUP(cache-libDir, libDir) 0)
goto error;
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 37841d1..cc45828 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -1102,12 +1102,6 @@ qemuCCWAdressIncrement(virDomainDeviceCCWAddressPtr addr)
return 0;
}
-static void
-qemuDomainCCWAddressSetFreeEntry(void *payload,
- const void *name ATTRIBUTE_UNUSED)
-{
-VIR_FREE(payload);
-}
int qemuDomainCCWAddressAssign(virDomainDeviceInfoPtr dev,
qemuDomainCCWAddressSetPtr addrs,
@@ -1264,7 +1258,7 @@ qemuDomainCCWAddressSetCreate(void)
if (VIR_ALLOC(addrs) 0)
goto error;
-if (!(addrs-defined = virHashCreate(10,
qemuDomainCCWAddressSetFreeEntry)))
+if (!(addrs-defined = virHashCreate(10, virHashValueFree)))
goto error;
/* must use cssid = 0xfe (254) for virtio-ccw devices */
diff --git a/src/qemu/qemu_monitor.c
Re: [libvirt] [PATCH] tests: build viridentitytest only WITH_ATTR.
On Fri, Apr 4, 2014 at 6:57 PM, Jincheng Miao jm...@redhat.com wrote: Commit d7c4e0036 assumed all SELinux tests depended upon securityselinuxhelper need xattr support, but forgot to move viridentitytest under WITH_ATTR. Reported-by: Nehal J Wani nehaljw.k...@gmail.com Signed-off-by: Jincheng Miao jm...@redhat.com --- tests/Makefile.am | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Makefile.am b/tests/Makefile.am index 6e15af8..7a15295 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -146,7 +146,6 @@ test_programs = virshtest sockettest \ virpcitest \ virendiantest \ virfiletest \ - viridentitytest \ viriscsitest \ virkeycodetest \ virlockspacetest \ @@ -188,7 +187,8 @@ endif WITH_DBUS if WITH_SECDRIVER_SELINUX if WITH_ATTR -test_programs += securityselinuxtest +test_programs += securityselinuxtest \ + viridentitytest if WITH_QEMU test_programs += securityselinuxlabeltest endif WITH_QEMU -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list ACK -- Nehal J Wani -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
