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" 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
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" 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] [PATCH] virsh: Fix help info of net-event and event command
On 04/04/2014 07:58 AM, liyang wrote: > From: Li Yang > > 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 > --- > 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 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] [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 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 > --- > 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] [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 --- 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] 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 > --- > 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
[libvirt] [PATCH] virsh: Fix help info of net-event and event command
From: Li Yang 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 --- 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] [RFC PATCH v2 1/3] qemu: Expose additional timing metrics for 'setup' and 'mbps'
On 02/03/2014 08:52 PM, Daniel P. Berrange wrote: On Mon, Feb 03, 2014 at 01:32:59PM +0100, Jiri Denemark wrote: On Mon, Jan 13, 2014 at 14:28:10 +0800, mrhi...@linux.vnet.ibm.com wrote: From: "Michael R. Hines" RDMA migration uses the 'setup' state in QEMU to optionally lock all memory before the migration starts. The total time spent in this state is already exposed by QEMU, so expose it in libvirt. Additionally, QEMU also now exports migration throughput (mbps), so let's see that one too... Signed-off-by: Michael R. Hines --- include/libvirt/libvirt.h.in | 15 +++ src/qemu/qemu_driver.c | 14 ++ src/qemu/qemu_monitor.h | 12 src/qemu/qemu_monitor_json.c | 8 4 files changed, 49 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5aad75c..5ac2694 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4153,6 +4153,21 @@ int virDomainAbortJob(virDomainPtr dom); #define VIR_DOMAIN_JOB_DOWNTIME "downtime" /** + * VIR_DOMAIN_JOB_MBPS: + * + * virDomainGetJobStats field: network throughput used while migrating. + */ +#define VIR_DOMAIN_JOB_MBPS "mbps" I think this would be better as #define VIR_DOMAIN_JOB_THROUGHPUT "throughput" And you need to explicitly mention the type of the value returned in this parameter. Hmm, in block I/O tuning XML/APIs we use either VIR_DOMAIN_BLOCK_IOTUNE_TOTAL_BYTES_SEC or VIR_DOMAIN_BLKIO_DEVICE_READ_BPS which would suggest that first we should measure this in bps too, not mbp, and using either BPS or BYTES_SEC as a constant suffix is reasonable for consistency. Daniel Acknowledged. - Michael -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [RFC PATCH v2 1/3] qemu: Expose additional timing metrics for 'setup' and 'mbps'
On 02/03/2014 08:32 PM, Jiri Denemark wrote: On Mon, Jan 13, 2014 at 14:28:10 +0800, mrhi...@linux.vnet.ibm.com wrote: From: "Michael R. Hines" RDMA migration uses the 'setup' state in QEMU to optionally lock all memory before the migration starts. The total time spent in this state is already exposed by QEMU, so expose it in libvirt. Additionally, QEMU also now exports migration throughput (mbps), so let's see that one too... Signed-off-by: Michael R. Hines --- include/libvirt/libvirt.h.in | 15 +++ src/qemu/qemu_driver.c | 14 ++ src/qemu/qemu_monitor.h | 12 src/qemu/qemu_monitor_json.c | 8 4 files changed, 49 insertions(+) diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 5aad75c..5ac2694 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -4153,6 +4153,21 @@ int virDomainAbortJob(virDomainPtr dom); #define VIR_DOMAIN_JOB_DOWNTIME "downtime" /** + * VIR_DOMAIN_JOB_MBPS: + * + * virDomainGetJobStats field: network throughput used while migrating. + */ +#define VIR_DOMAIN_JOB_MBPS "mbps" I think this would be better as #define VIR_DOMAIN_JOB_THROUGHPUT "throughput" And you need to explicitly mention the type of the value returned in this parameter. Acknowledged. + +/** + * VIR_DOMAIN_JOB_SETUP_TIME: + * + * virDomainGetJobStats field: total time in milliseconds spent preparing + * the migration in the 'setup' phase before the iterations begin. + */ +#define VIR_DOMAIN_JOB_SETUP_TIME "setup_time" Again, the documentation does not mention what type is returned. Acknowledged. + +/** * VIR_DOMAIN_JOB_DATA_TOTAL: * * virDomainGetJobStats field: total number of bytes supposed to be diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 47d8a09..6a7dcc7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11078,6 +11078,20 @@ qemuDomainGetJobStats(virDomainPtr dom, goto cleanup; } +if (priv->job.status.mbps_set) { +if (virTypedParamsAddDouble(&par, &npar, &maxpar, +VIR_DOMAIN_JOB_MBPS, +priv->job.status.mbps) < 0) +goto cleanup; +} I think we should take the Mbps value from qemu and turn it into Bps (or even bps), which would make this parameter consistent with other migration statistics parameters that return values in bytes. And then we can change the type to ULLong. Acknowledged. + +if (priv->job.status.setup_time_set) { +if (virTypedParamsAddULLong(&par, &npar, &maxpar, +VIR_DOMAIN_JOB_SETUP_TIME, +priv->job.status.setup_time) < 0) +goto cleanup; +} + if (virTypedParamsAddULLong(&par, &npar, &maxpar, VIR_DOMAIN_JOB_DISK_TOTAL, priv->job.info.fileTotal) < 0 || diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index eabf000..27f9cb4 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -412,6 +412,18 @@ struct _qemuMonitorMigrationStatus { /* total or expected depending on status */ bool downtime_set; unsigned long long downtime; +/* + * Duration of the QEMU 'setup' state. + * for RDMA, this may be on the order of several seconds + * if pinning support is requested before the migration begins. + */ +bool setup_time_set; +unsigned long long setup_time; +/* + * Migration throughput in Mbps. + */ +bool mbps_set; +double mbps; unsigned long long ram_transferred; unsigned long long ram_remaining; diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index ec3b958..214e140 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -2426,6 +2426,14 @@ qemuMonitorJSONGetMigrationStatusReply(virJSONValuePtr reply, virJSONValueObjectGetNumberUlong(ram, "normal-bytes", &status->ram_normal_bytes); +if (virJSONValueObjectGetNumberDouble(ram, "mbps", + &status->mbps) == 0) +status->mbps_set = true; + +if (virJSONValueObjectGetNumberUlong(ret, "setup-time", + &status->setup_time) == 0) +status->setup_time_set = true; + virJSONValuePtr disk = virJSONValueObjectGet(ret, "disk"); if (disk) { rc = virJSONValueObjectGetNumberUlong(disk, "transferred", Overall the patch looks good and could even be pushed separately. Jirka Will submit a v3 for everything soon - Michae -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 5/5] conf: start testing contents of the new backing chain fields
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 --- tests/virstoragetest.c | 239 - 1 file changed, 175 insertions(+), 64 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index efd920a..9d6e344 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -205,6 +205,7 @@ struct _testFileData bool expIsFile; unsigned long long expCapacity; bool expEncrypted; +const char *expPath; }; enum { @@ -266,21 +267,25 @@ testStorageChain(const void *args) } if (virAsprintf(&expect, -"store:%s\nraw:%s\ndirectory:%s\nother:%d %d %lld %d", +"store:%s\nraw:%s\ndirectory:%s\nother:%d %d %lld %d\n" +"path:%s\n", NULLSTR(data->files[i]->expBackingStore), NULLSTR(data->files[i]->expBackingStoreRaw), NULLSTR(data->files[i]->expDirectory), data->files[i]->expFormat, data->files[i]->expIsFile, data->files[i]->expCapacity, -data->files[i]->expEncrypted) < 0 || +data->files[i]->expEncrypted, +NULLSTR(data->files[i]->expPath)) < 0 || virAsprintf(&actual, -"store:%s\nraw:%s\ndirectory:%s\nother:%d %d %lld %d", +"store:%s\nraw:%s\ndirectory:%s\nother:%d %d %lld %d\n" +"path:%s\n", NULLSTR(elt->backingStore), NULLSTR(elt->backingStoreRaw), NULLSTR(elt->directory), elt->backingStoreFormat, elt->backingStoreIsFile, -elt->capacity, !!elt->encryption) < 0) { +elt->capacity, !!elt->encryption, +NULLSTR(elt->path)) < 0) { VIR_FREE(expect); VIR_FREE(actual); goto cleanup; @@ -351,9 +356,24 @@ mymain(void) } while (0) /* Expected details about files in chains */ -const testFileData raw = { +const testFileData rel_raw = { .expFormat = VIR_STORAGE_FILE_NONE, +.expPath = "raw", }; +const testFileData abs_raw = { +.expFormat = VIR_STORAGE_FILE_NONE, +.expPath = canonraw, +}; + +const testFileData rel_qcow2_as_raw = { +.expFormat = VIR_STORAGE_FILE_NONE, +.expPath = "qcow2", +}; +const testFileData abs_qcow2_as_raw = { +.expFormat = VIR_STORAGE_FILE_NONE, +.expPath = canonqcow2, +}; + const testFileData qcow2_relback_relstart = { .expBackingStore = canonraw, .expBackingStoreRaw = "raw", @@ -361,6 +381,7 @@ mymain(void) .expFormat = VIR_STORAGE_FILE_RAW, .expIsFile = true, .expCapacity = 1024, +.expPath = "qcow2", }; const testFileData qcow2_relback_absstart = { .expBackingStore = canonraw, @@ -369,15 +390,28 @@ mymain(void) .expFormat = VIR_STORAGE_FILE_RAW, .expIsFile = true, .expCapacity = 1024, +.expPath = canonqcow2, }; -const testFileData qcow2_absback = { + +const testFileData rel_qcow2_absback = { .expBackingStore = canonraw, .expBackingStoreRaw = absraw, .expDirectory = datadir, .expFormat = VIR_STORAGE_FILE_RAW, .expIsFile = true, .expCapacity = 1024, +.expPath = "qcow2", }; +const testFileData abs_qcow2_absback = { +.expBackingStore = canonraw, +.expBackingStoreRaw = absraw, +.expDirectory = datadir, +.expFormat = VIR_STORAGE_FILE_RAW, +.expIsFile = true, +.expCapacity = 1024, +.expPath = canonqcow2, +}; + const testFileData qcow2_as_probe = { .expBackingStore = canonraw, .expBackingStoreRaw = absraw, @@ -385,50 +419,122 @@ mymain(void) .expFormat = VIR_STORAGE_FILE_AUTO, .expIsFile = true, .expCapacity = 1024, +.expPath = canonqcow2, }; -const testFileData qcow2_bogus = { + +const testFileData rel_qcow2_bogus = { .expBackingStoreRaw = datadir "/bogus", .expDirectory = datadir, .expFormat = VIR_STORAGE_FILE_NONE, .expCapacity = 1024, +.expPath = "qcow2", }; -const testFileData qcow2_protocol = { +const t
[libvirt] [PATCH 4/5] conf: track more fields in backing chain metadata
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 --- 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 @@ -798,6 +798,10 @@ virStorageFileGetMetadataInternal(const char *path, if (VIR_ALLOC(meta) < 0) return NULL; +if (VIR_STRDUP(meta->path, path) < 0) +goto cleanup; +if (VIR_STRDUP(meta->relDir, directory) < 0) +goto cleanup; if (format == VIR_STORAGE_FILE_AUTO) format = virStorageFileProbeFormatFromBuf(path, buf, len); @@ -808,6 +812,7 @@ virStorageFileGetMetadataInternal(const char *path, format); goto cleanup; } +meta->format = format; /* XXX we should consider moving virStorageBackendUpdateVolInfo * code into this method, for non-magic files @@ -1013,9 +1018,20 @@ virStorageFileGetMetadataFromFDInternal(const char *path, return NULL; } -/* No header to probe for directories, but also no backing file */ if (S_ISDIR(sb.st_mode)) { -ignore_value(VIR_ALLOC(ret)); +/* No header to probe for directories, but also no backing + * file; therefore, no inclusion loop is possible, and we + * don't need canonName or relDir. */ +if (VIR_ALLOC(ret) < 0) +goto cleanup; +if (VIR_STRDUP(ret->path, path) < 0) { +virStorageFileFreeMetadata(ret); +ret = NULL; +goto cleanup; +} +ret->type = VIR_STORAGE_TYPE_DIR; +ret->format = format > VIR_STORAGE_FILE_NONE ? format +: VIR_STORAGE_FILE_DIR; goto cleanup; } @@ -1030,6 +1046,12 @@ virStorageFileGetMetadataFromFDInternal(const char *path, } ret = virStorageFileGetMetadataInternal(path, buf, len, directory, format); +if (ret) { +if (S_ISREG(sb.st_mode)) +ret->type = VIR_STORAGE_TYPE_FILE; +else if (S_ISBLK(sb.st_mode)) +ret->type = VIR_STORAGE_TYPE_BLOCK; +} cleanup: VIR_FREE(buf); 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; + +/* Backing chain. backingMeta->origName should match + *
[libvirt] [PATCH 3/5] conf: track when storage type is still undetermined
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 --- 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(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 465bf84..0c5c7ab 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4965,7 +4965,7 @@ virDomainDiskSourceParse(xmlNodePtr node, memset(&host, 0, sizeof(host)); -switch (src->type) { +switch ((enum virStorageType)src->type) { case VIR_STORAGE_TYPE_FILE: src->path = virXMLPropString(node, "file"); break; @@ -5053,7 +5053,8 @@ virDomainDiskSourceParse(xmlNodePtr node, if (virDomainDiskSourcePoolDefParse(node, &src->srcpool) < 0) goto cleanup; break; -default: +case VIR_STORAGE_TYPE_NONE: +case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %s"), virStorageTypeToString(src->type)); @@ -5150,7 +5151,7 @@ virDomainDiskDefParseXML(virDomainXMLOptionPtr xmlopt, type = virXMLPropString(node, "type"); if (type) { -if ((def->src.type = virStorageTypeFromString(type)) < 0) { +if ((def->src.type = virStorageTypeFromString(type)) <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("unknown disk type '%s'"), type); goto error; @@ -14836,6 +14837,7 @@ virDomainDiskSourceFormat(virBufferPtr buf, src->seclabels, flags); break; +case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %d"), src->type); @@ -14867,7 +14869,7 @@ virDomainDiskDefFormat(virBufferPtr buf, char uuidstr[VIR_UUID_STRING_BUFLEN]; -if (!type) { +if (!type || !def->src.type) { virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected disk type %d"), def->src.type); return -1; diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c index 374a104..852a286 100644 --- a/src/conf/snapshot_conf.c +++ b/src/conf/snapshot_conf.c @@ -132,7 +132,7 @@ virDomainSnapshotDiskDefParseXML(xmlNodePtr node, } if ((type = virXMLPropString(node, "type"))) { -if ((def->src.type = virStorageTypeFromString(type)) < 0 || +if ((def->src.type = virStorageTypeFromString(type)) <= 0 || def->src.type == VIR_STORAGE_TYPE_VOLUME || def->src.type == VIR_STORAGE_TYPE_DIR) { virReportError(VIR_ERR_XML_ERROR, diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 099a777..37841d1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3856,6 +3856,7 @@ qemuGetDriveSourceString(int type, break; case VIR_STORAGE_TYPE_VOLUME: +case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: break; } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1d08951..4bb4819 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12356,6 +12356,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) case VIR_STORAGE_TYPE_DIR: case VIR_STORAGE_TYPE_VOLUME: +case VIR_STORAGE_TYPE_NONE: case VIR_STORAGE_TYPE_LAST: virReportError(VIR_ERR_INTERNAL_ERROR, _("external inactive snapshots are not supported on " @@ -12420,6 +12421,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSna
[libvirt] [PATCH 2/5] tests: refactor virstoragetest for less stack space
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. * 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 --- tests/virstoragetest.c | 167 - 1 file changed, 81 insertions(+), 86 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 9a3b3a3..efd920a 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -218,7 +218,7 @@ struct testChainData { const char *start; enum virStorageFileFormat format; -const testFileData *files; +const testFileData *files[5]; int nfiles; unsigned int flags; }; @@ -267,13 +267,13 @@ testStorageChain(const void *args) if (virAsprintf(&expect, "store:%s\nraw:%s\ndirectory:%s\nother:%d %d %lld %d", -NULLSTR(data->files[i].expBackingStore), -NULLSTR(data->files[i].expBackingStoreRaw), -NULLSTR(data->files[i].expDirectory), -data->files[i].expFormat, -data->files[i].expIsFile, -data->files[i].expCapacity, -data->files[i].expEncrypted) < 0 || +NULLSTR(data->files[i]->expBackingStore), +NULLSTR(data->files[i]->expBackingStoreRaw), +NULLSTR(data->files[i]->expDirectory), +data->files[i]->expFormat, +data->files[i]->expIsFile, +data->files[i]->expCapacity, +data->files[i]->expEncrypted) < 0 || virAsprintf(&actual, "store:%s\nraw:%s\ndirectory:%s\nother:%d %d %lld %d", NULLSTR(elt->backingStore), @@ -312,29 +312,42 @@ mymain(void) { int ret; virCommandPtr cmd = NULL; +struct testChainData data; /* Prep some files with qemu-img; if that is not found on PATH, or * if it lacks support for qcow2 and qed, skip this test. */ if ((ret = testPrepImages()) != 0) return ret; -#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, \ +size_t i;\ +memset(&data, 0, sizeof(data)); \ +data = (struct testChainData){ \ +start, format, { __VA_ARGS__ }, 0, flags,\ }; \ +for (i = 0; i < ARRAY_CARDINALITY(data.files); i++) \ +if (data.files[i]) \ +data.nfiles++; \ if (virtTestRun("Storage backing chain " id, \ testStorageChain, &data) < 0)\ ret = -1;\ } while (0) +#define VIR_FLATTEN_2(...) __VA_ARGS__ +#define VIR_FLATTEN_1(_1) VIR_FLATTEN_2 _1 + #define TEST_CHAIN(id, relstart, absstart, format, chain1, flags1, \ chain2, flags2, chain3, flags3, chain4, flags4) \ do { \ -TEST_ONE_CHAIN(#id "a", relstart, format, chain1, flags1); \ -TEST_ONE_CHAIN(#id "b", relstart, format, chain2, flags2); \ -TEST_ONE_CHAIN(#id "c", absstart, format, chain3, flags3); \ -TEST_ONE_CHAIN(#id "d", absstart, format, chain4, flags4); \ +TEST_ONE_CHAIN(#id "a", relstart, format, flags1,\ + VIR_FLATTEN_1(chain1)); \ +TEST_ONE_CHAIN(#id "b"
[libvirt] [PATCH 0/5] virstoragefile refactoring, part 3
Part 2 gave us a common virStorageSource struct, now it's time to start using that struct when crawling backing file chains. I posted some RFCs about my full conversion plan, here's the patches I have working so far. Still plenty more to come, but today's batch of patches took me a lot longer than I had planned due to having to refactor the testsuite to avoid a compiler error about over-large stack allocation. Eric Blake (5): tests: use C99 initialization for storage test tests: refactor virstoragetest for less stack space conf: track when storage type is still undetermined conf: track more fields in backing chain metadata conf: start testing contents of the new backing chain fields 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 | 33 +++- src/util/virstoragefile.h | 43 - tests/virstoragetest.c| 405 -- 7 files changed, 366 insertions(+), 139 deletions(-) -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/5] tests: use C99 initialization for storage test
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 --- tests/virstoragetest.c | 105 + 1 file changed, 80 insertions(+), 25 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 9ec39c7..9a3b3a3 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -339,60 +339,115 @@ mymain(void) /* Expected details about files in chains */ const testFileData raw = { -NULL, NULL, NULL, VIR_STORAGE_FILE_NONE, false, 0, false, +.expFormat = VIR_STORAGE_FILE_NONE, }; const testFileData qcow2_relback_relstart = { -canonraw, "raw", ".", VIR_STORAGE_FILE_RAW, true, 1024, false, +.expBackingStore = canonraw, +.expBackingStoreRaw = "raw", +.expDirectory = ".", +.expFormat = VIR_STORAGE_FILE_RAW, +.expIsFile = true, +.expCapacity = 1024, }; const testFileData qcow2_relback_absstart = { -canonraw, "raw", datadir, VIR_STORAGE_FILE_RAW, true, 1024, false, +.expBackingStore = canonraw, +.expBackingStoreRaw = "raw", +.expDirectory = datadir, +.expFormat = VIR_STORAGE_FILE_RAW, +.expIsFile = true, +.expCapacity = 1024, }; const testFileData qcow2_absback = { -canonraw, absraw, datadir, VIR_STORAGE_FILE_RAW, true, 1024, false, +.expBackingStore = canonraw, +.expBackingStoreRaw = absraw, +.expDirectory = datadir, +.expFormat = VIR_STORAGE_FILE_RAW, +.expIsFile = true, +.expCapacity = 1024, }; const testFileData qcow2_as_probe = { -canonraw, absraw, datadir, VIR_STORAGE_FILE_AUTO, true, 1024, false, +.expBackingStore = canonraw, +.expBackingStoreRaw = absraw, +.expDirectory = datadir, +.expFormat = VIR_STORAGE_FILE_AUTO, +.expIsFile = true, +.expCapacity = 1024, }; const testFileData qcow2_bogus = { -NULL, datadir "/bogus", datadir, VIR_STORAGE_FILE_NONE, -false, 1024, false, +.expBackingStoreRaw = datadir "/bogus", +.expDirectory = datadir, +.expFormat = VIR_STORAGE_FILE_NONE, +.expCapacity = 1024, }; const testFileData qcow2_protocol = { -"nbd:example.org:6000", NULL, NULL, VIR_STORAGE_FILE_RAW, -false, 1024, false, +.expBackingStore = "nbd:example.org:6000", +.expFormat = VIR_STORAGE_FILE_RAW, +.expCapacity = 1024, }; const testFileData wrap = { -canonqcow2, absqcow2, datadir, VIR_STORAGE_FILE_QCOW2, -true, 1024, false, +.expBackingStore = canonqcow2, +.expBackingStoreRaw = absqcow2, +.expDirectory = datadir, +.expFormat = VIR_STORAGE_FILE_QCOW2, +.expIsFile = true, +.expCapacity = 1024, }; const testFileData wrap_as_raw = { -canonqcow2, absqcow2, datadir, VIR_STORAGE_FILE_RAW, -true, 1024, false, +.expBackingStore = canonqcow2, +.expBackingStoreRaw = absqcow2, +.expDirectory = datadir, +.expFormat = VIR_STORAGE_FILE_RAW, +.expIsFile = true, +.expCapacity = 1024, }; const testFileData wrap_as_probe = { -canonqcow2, absqcow2, datadir, VIR_STORAGE_FILE_AUTO, -true, 1024, false, +.expBackingStore = canonqcow2, +.expBackingStoreRaw = absqcow2, +.expDirectory = datadir, +.expFormat = VIR_STORAGE_FILE_AUTO, +.expIsFile = true, +.expCapacity = 1024, }; const testFileData qed = { -canonraw, absraw, datadir, VIR_STORAGE_FILE_RAW, -true, 1024, false, +.expBackingStore = canonraw, +.expBackingStoreRaw = absraw, +.expDirectory = datadir, +.expFormat = VIR_STORAGE_FILE_RAW, +.expIsFile = true, +.expCapacity = 1024, }; #if HAVE_SYMLINK const testFileData link1_rel = { -canonraw, "../raw", "sub/../sub/..", VIR_STORAGE_FILE_RAW, -true, 1024, false, +.expBackingStore = canonraw, +.expBackingStoreRaw = "../raw", +.expDirectory = "sub/../sub/..", +.expFormat = VIR_STORAGE_FILE_RAW, +.expIsFile = true, +.expCapacity = 1024, }; const testFileData link1_abs = { -canonraw, "../raw", datadir "/sub/../sub/..", VIR_STORAGE_FILE_RAW, -true, 1024, false, +.expBackingStore = canonraw, +.expBackingStoreRaw = "../raw", +.expDirectory = datadir "/sub/../sub/..", +.expFormat = VIR_STORAGE_FILE_RAW, +.expIsFile = true, +.expCapacity = 1024, }; const testFileData link2_rel = { -canonqcow2, "../sub/link1", "sub/../sub", VIR_STORAGE_FILE_QCOW2, -true, 1024, false, +
[libvirt] [PATCH v2] Define CPUINFO_FILE_LEN and fix maxlen of cpuinfo file for all uses
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 --- 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*/ + #endif /* __VIR_SYSINFOS_H__ */ -- 1.8.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Define CPUINFO_FILE_LEN and fix maxlen of cpuinfo file for powerpc
Hi Eric, Thanks for comments. I'll submit a new revision of this patch. Best Regards, Olivia > -Original Message- > From: Eric Blake [mailto:ebl...@redhat.com] > Sent: Thursday, April 03, 2014 8:25 PM > To: Yin Olivia-R63875; libvir-list@redhat.com > Subject: Re: [libvirt] [PATCH] Define CPUINFO_FILE_LEN and fix maxlen of > cpuinfo file for powerpc > > On 04/03/2014 12:43 AM, Olivia Yin wrote: > > This patch define CPUINFO_FILE_LEN as 2KB which is enough for most > architectures. > > > > For the 24 cores PowerPC machines, the file of /proc/cpuinfo will be > larger than 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 > > > > So this patch increases the maxlen of cpuinfo for PowerPC architecture. > > > > Signed-off-by: Olivia Yin > > --- > > 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..33c4bc6 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, 2 * CPUINFO_FILE_LEN, &outbuf) < 0) { > > Uggh. Having to multiply the constant means you defined the constant wrong. > Just make CPUINFO_FILE_LEN big enough. Even as large as 1M, for ALL uses, > is not a burden. > > > +++ b/src/util/virsysinfo.h > > @@ -102,4 +102,6 @@ bool virSysinfoIsEqual(virSysinfoDefPtr src, > > > > VIR_ENUM_DECL(virSysinfo) > > > > +#define CPUINFO_FILE_LEN 2048/*2KB limit for normal cpuinfo file*/ > > + > > #endif /* __VIR_SYSINFOS_H__ */ > > > > -- > Eric Blake eblake redhat com+1-919-301-3266 > Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Enhancing clean-traffic to work with IPv6
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. Any suggestions? -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix Memory Leak in daemon/libvirtd.c
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; 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 */ } -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v1 0/2] Keep original file label
On 13.03.2014 10:02, Michal Privoznik wrote: This creates a new file to store the original file labels. Or do we want the whole functionality to reside in virtlockd? Michal Privoznik (2): security_dac: Keep original file label security_dac: Lock label store file src/Makefile.am | 3 +- src/locking/lock_driver.h| 2 + src/locking/lock_driver_lockd.c | 25 +++ src/lxc/lxc_controller.c | 2 +- src/lxc/lxc_driver.c | 3 +- src/qemu/qemu_driver.c | 9 +- src/security/security_dac.c | 412 +++ src/security/security_manager.c | 24 ++- src/security/security_manager.h | 7 +- tests/qemuhotplugtest.c | 2 +- tests/seclabeltest.c | 2 +- tests/securityselinuxlabeltest.c | 3 +- tests/securityselinuxtest.c | 2 +- 13 files changed, 438 insertions(+), 58 deletions(-) Ping? Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libxl fixes/improvements for libvirt
On 03.04.2014 17:45, Michal Privoznik wrote: > On 27.03.2014 17:55, Stefan Bader wrote: >> Here several changes which improve the handling of Xen for me: >> >> * 0001-libxl-Use-id-from-virDomainObj-inside-the-driver.patch >>This is a re-send as I initially submitted that as a reply to some >>discussion. Starting from the visibly broken libxlDomainGetInfo when >>creating or rebooting a guest with virt-manager, I replaced all usage >>of dom->id with the more stable vm->def->id. >> * 0002-libxl-Set-disk-format-for-empty-cdrom-device.patch >>Fixes start of a guest after ejecting the iso image from a virtual >>CDROM drive (again using virt-manager). >> * 0003-libxl-Implement-basic-video-device-selection.patch >>Should fix the occasional disagreement about the used VNC port and >>adds the ability to select the standard VGA graphics from Xen. >>VRAM size seemed to work with that. Only for Cirrus, while the qemu >>command-line looks good, ones seems to end up with 32M. >> >> Note to people on the Xen list: I wonder whether libxenlight internally >> should somehow should fix up the situation where a caller sets up the >> video devices in the vfbs list but does not sync that with the information >> in the build info. Question probably is what the semantics should be. >> >> -Stefan >> > > Just for the record, I've pushed the first two patches. The last one has some > issues, so I rather see it's second version. > > Michal > Ack, thanks. I will send out an updated version as soon as I get it done. Next on my list though I cannot exactly tell when "next" happens. :) -Stefan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libxl fixes/improvements for libvirt
On 27.03.2014 17:55, Stefan Bader wrote: Here several changes which improve the handling of Xen for me: * 0001-libxl-Use-id-from-virDomainObj-inside-the-driver.patch This is a re-send as I initially submitted that as a reply to some discussion. Starting from the visibly broken libxlDomainGetInfo when creating or rebooting a guest with virt-manager, I replaced all usage of dom->id with the more stable vm->def->id. * 0002-libxl-Set-disk-format-for-empty-cdrom-device.patch Fixes start of a guest after ejecting the iso image from a virtual CDROM drive (again using virt-manager). * 0003-libxl-Implement-basic-video-device-selection.patch Should fix the occasional disagreement about the used VNC port and adds the ability to select the standard VGA graphics from Xen. VRAM size seemed to work with that. Only for Cirrus, while the qemu command-line looks good, ones seems to end up with 32M. Note to people on the Xen list: I wonder whether libxenlight internally should somehow should fix up the situation where a caller sets up the video devices in the vfbs list but does not sync that with the information in the build info. Question probably is what the semantics should be. -Stefan Just for the record, I've pushed the first two patches. The last one has some issues, so I rather see it's second version. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 5/5] virsh: Expose new virDomainFSFreeze and virDomainFSThaw API
These are exposed under domfsfreeze command and domfsthaw command. Signed-off-by: Tomoki Sekiyama --- tools/virsh-domain.c | 128 ++ tools/virsh.pod | 23 + 2 files changed, 151 insertions(+) diff --git a/tools/virsh-domain.c b/tools/virsh-domain.c index 73414f8..331ba36 100644 --- a/tools/virsh-domain.c +++ b/tools/virsh-domain.c @@ -11391,6 +11391,122 @@ cmdDomFSTrim(vshControl *ctl, const vshCmd *cmd) return ret; } +static const vshCmdInfo info_domfsfreeze[] = { +{.name = "help", + .data = N_("Freeze domain's mounted filesystems.") +}, +{.name = "desc", + .data = N_("Freeze domain's mounted filesystems.") +}, +{.name = NULL} +}; + +static const vshCmdOptDef opts_domfsfreeze[] = { +{.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") +}, +{.name = "disks", + .type = VSH_OT_DATA, + .help = N_("comma separated list of disks to be frozen") +}, +{.name = NULL} +}; +static bool +cmdDomFSFreeze(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +bool ret = false; +unsigned int flags = 0; +const char *disk_string = NULL; +char **disk_list = NULL; /* tokenized disk_string */ +int ndisks = 0; +size_t i; + +ignore_value(vshCommandOptString(cmd, "disks", &disk_string)); + +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +return ret; + +if (disk_string) { +if ((ndisks = vshStringToArray(disk_string, &disk_list)) < 0) +goto cleanup; +} + +if (virDomainFSFreeze(dom, (const char **)disk_list, ndisks, flags) < 0) { +vshError(ctl, _("Unable to freeze filesystems")); +goto cleanup; +} + +ret = true; + + cleanup: +for (i = 0; i < ndisks; i++) +VIR_FREE(disk_list[i]); +VIR_FREE(disk_list); +virDomainFree(dom); +return ret; +} + +static const vshCmdInfo info_domfsthaw[] = { +{.name = "help", + .data = N_("Thaw domain's mounted filesystems.") +}, +{.name = "desc", + .data = N_("Thaw domain's mounted filesystems.") +}, +{.name = NULL} +}; + +static const vshCmdOptDef opts_domfsthaw[] = { +{.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") +}, +{.name = "disks", + .type = VSH_OT_DATA, + .help = N_("comma separated list of disks to be thawed") +}, +{.name = NULL} +}; +static bool +cmdDomFSThaw(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom = NULL; +bool ret = false; +unsigned int flags = 0; +const char *disk_string = NULL; +char **disk_list = NULL; /* tokenized disk_string */ +int ndisks = 0; +size_t i; + +ignore_value(vshCommandOptString(cmd, "disks", &disk_string)); + +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +return ret; + +if (disk_string) { +if ((ndisks = vshStringToArray(disk_string, &disk_list)) < 0) +goto cleanup; +} + +if (virDomainFSThaw(dom, (const char **)disk_list, ndisks, flags) < 0) { +vshError(ctl, _("Unable to thaw filesystems")); +goto cleanup; +} + +ret = true; + + cleanup: +for (i = 0; i < ndisks; i++) +VIR_FREE(disk_list[i]); +VIR_FREE(disk_list); +virDomainFree(dom); +return ret; +} + const vshCmdDef domManagementCmds[] = { {.name = "attach-device", .handler = cmdAttachDevice, @@ -11538,6 +11654,18 @@ const vshCmdDef domManagementCmds[] = { .info = info_domdisplay, .flags = 0 }, +{.name = "domfsfreeze", + .handler = cmdDomFSFreeze, + .opts = opts_domfsfreeze, + .info = info_domfsfreeze, + .flags = 0 +}, +{.name = "domfsthaw", + .handler = cmdDomFSThaw, + .opts = opts_domfsthaw, + .info = info_domfsthaw, + .flags = 0 +}, {.name = "domfstrim", .handler = cmdDomFSTrim, .opts = opts_domfstrim, diff --git a/tools/virsh.pod b/tools/virsh.pod index 98d891a..652aabc 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -941,6 +941,29 @@ Output a URI which can be used to connect to the graphical display of the domain via VNC, SPICE or RDP. If I<--include-password> is specified, the SPICE channel password will be included in the URI. +=item B I [I<--disks> B] + +Freeze all mounted filesystems within a running domain to prepare for +consistent snapshots. + +The I<--disks> flag takes a parameter B, which is a comma separated +list of names of disks to be frozen. If this is not specified, every mounted +filesystem on the guest is frozen. + +Note that B command has a I<--quiesce> option to freeze +and thaw the filesystems automatically to keep snapshots consistent. +B command is only needed when a user wants to utilize the +native snapshot features of storage devices not supported by libvirt yet. + +=item B I [I<--disks> B] +
[libvirt] [PATCH v5 3/5] qemu: Track domain quiesced status and make fsfreeze/thaw nestable
Adds 'quiesced' counter into qemuDomainObjPrivate that tracks how many times fsfreeze is requested in the domain. When qemuDomainSnapshotFSFreeze is called, the counter is incremented. The fsfreeze request is sent to the guest agent when the counter changes from 0 to 1. qemuDomainSnapshotFSThaw decrements the counter and requests fsthaw when it becomes 0. It also modify error code from qemuDomainSnapshotFSFreeze and qemuDomainSnapshotFSThaw, so that a caller can know whether the command is actually sent to the guest agent. If the error is caused before sending a freeze command, a counterpart thaw command shouldn't be sent either. Signed-off-by: Tomoki Sekiyama --- src/qemu/qemu_domain.c |6 src/qemu/qemu_domain.h |2 + src/qemu/qemu_driver.c | 65 ++-- 3 files changed, 59 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index cdd4601..6ed3d67 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -357,6 +357,9 @@ qemuDomainObjPrivateXMLFormat(virBufferPtr buf, void *data) virBufferAddLit(buf, "\n"); } +if (priv->quiesced) +virBufferAsprintf(buf, "\n", priv->quiesced); + return 0; } @@ -518,6 +521,9 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, void *data) } VIR_FREE(nodes); +if (virXPathInt("string(./quiesced/@depth)", ctxt, &priv->quiesced) < 0) +priv->quiesced = 0; + return 0; error: diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index b2830c4..620a258 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -176,6 +176,8 @@ struct _qemuDomainObjPrivate { char **qemuDevices; /* NULL-terminated list of devices aliases known to QEMU */ bool hookRun; /* true if there was a hook run over this domain */ + +int quiesced; /* count how many times filesystems are quiesced */ }; typedef enum { diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 9a2de12..2bb5fa9 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12100,32 +12100,68 @@ qemuDomainPrepareDiskChainElement(virQEMUDriverPtr driver, } +/* Return -1 if request is not sent to agent due to misconfig, -2 if request + * is sent but failed, and number of frozen filesystems on success. */ static int -qemuDomainSnapshotFSFreeze(virDomainObjPtr vm) +qemuDomainSnapshotFSFreeze(virQEMUDriverPtr driver, virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; +virQEMUDriverConfigPtr cfg; int freezed; if (!qemuDomainAgentAvailable(priv, true)) return -1; +priv->quiesced++; +cfg = virQEMUDriverGetConfig(driver); +if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) { +virObjectUnref(cfg); +priv->quiesced--; +return -1; +} +virObjectUnref(cfg); + +if (priv->quiesced > 1) +return 0; + qemuDomainObjEnterAgent(vm); freezed = qemuAgentFSFreeze(priv->agent); qemuDomainObjExitAgent(vm); - -return freezed; +return freezed < 0 ? -2 : freezed; } +/* Return -1 if request is not sent to agent due to misconfig, -2 if request + * is sent but failed, and number of thawed filesystems on success. */ static int -qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report) +qemuDomainSnapshotFSThaw(virQEMUDriverPtr driver, + virDomainObjPtr vm, bool report) { qemuDomainObjPrivatePtr priv = vm->privateData; +virQEMUDriverConfigPtr cfg; int thawed; virErrorPtr err = NULL; if (!qemuDomainAgentAvailable(priv, report)) return -1; +if (!priv->quiesced && report) { +virReportError(VIR_ERR_OPERATION_INVALID, "%s", + _("domain is not quiesced")); +return -1; +} + +priv->quiesced--; +cfg = virQEMUDriverGetConfig(driver); +if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm) < 0) { +virObjectUnref(cfg); +priv->quiesced++; +return -1; +} +virObjectUnref(cfg); + +if (priv->quiesced) +return 0; + qemuDomainObjEnterAgent(vm); if (!report) err = virSaveLastError(); @@ -12135,7 +12171,7 @@ qemuDomainSnapshotFSThaw(virDomainObjPtr vm, bool report) qemuDomainObjExitAgent(vm); virFreeError(err); -return thawed; +return thawed < 0 ? -2 : thawed; } /* The domain is expected to be locked and inactive. */ @@ -13116,17 +13152,18 @@ qemuDomainSnapshotCreateActiveExternal(virConnectPtr conn, goto cleanup; /* If quiesce was requested, then issue a freeze command, and a - * counterpart thaw command, no matter what. The command will - * fail if the guest is paused or the guest agent is not - * running. */ + * counterpart thaw command when it is actually sent to agent. + * The command will fail if the guest is paused or the guest agent + * is not run
[libvirt] [PATCH v5 1/5] Introduce virDomainFSFreeze() and virDomainFSThaw() public API
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 --- 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); + /** * virSchedParameterType: * diff --git a/src/driver.h b/src/driver.h index e66fc7a..5c81d96 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1149,6 +1149,18 @@ typedef int unsigned int flags, int cancelled); +typedef int +(*virDrvDomainFSFreeze)(virDomainPtr dom, +const char **disks, +unsigned int ndisks, +unsigned int flags); + +typedef int +(*virDrvDomainFSThaw)(virDomainPtr dom, + const char **disks, + unsigned int ndisks, + unsigned int flags); + typedef struct _virDriver virDriver; typedef virDriver *virDriverPtr; @@ -1363,6 +1375,8 @@ struct _virDriver { virDrvDomainMigrateFinish3Params domainMigrateFinish3Params; virDrvDomainMigrateConfirm3Params domainMigrateConfirm3Params; virDrvConnectGetCPUModelNames connectGetCPUModelNames; +virDrvDomainFSFreeze domainFSFreeze; +virDrvDomainFSThaw domainFSThaw; }; diff --git a/src/libvirt.c b/src/libvirt.c index 4454829..43614b5 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20658,3 +20658,95 @@ virDomainFSTrim(virDomainPtr dom, virDispatchError(dom->conn); return -1; } + +/** + * virDomainFSFreeze: + * @dom: a domain object + * @disks: list of disk names to be frozen + * @ndisks: the number of disks specified in @disks + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Freeze filesystems on the specified disks within the guest (hence guest + * agent may be required depending on hypervisor used). If @ndisks is 0, + * every mounted filesystem on the guest is frozen. + * Freeze can be nested. When it is called on the same disk multiple times, + * the disk will not be thawed until virDomainFSThaw is called the same times. + * + * Returns 0 on success, -1 otherwise. + */ +int +virDomainFSFreeze(virDomainPtr dom, + const char **disks, + unsigned int ndisks, + unsigned int flags) +{ +VIR_DOMAIN_DEBUG(dom, "disks=%p, ndisks=%d, flags=%x", + disks, ndisks, flags); + +virResetLastError(); + +virCheckDomainReturn(dom, -1); +virCheckReadOnlyGoto(dom->conn->flags, error); +if (ndisks) +virCheckNonNullArgGoto(disks, error); +else +virCheckNullArgGoto(disks, error); + +if (dom->conn->driver->domainFSFreeze) { +int ret = dom->conn->driver->domainFSFreeze(dom, disks, ndisks, flags); +if (ret < 0) +goto error; +return ret; +} + +virReportUnsupportedError(); + + error: +virDispatchError(dom->conn); +return -1; +} + +/** + * virDomainFSThaw: + * @dom: a domain object + * @disks: list of disk names to be thawed + * @ndisks: the number of disks specified in @disks + * @flags: extra flags, not used yet, so callers should always pass 0 + * + * Thaw filesystems on the specified disks within the guest (hence guest + * agent may be required depending on hypervisor used). If @ndisks is 0, + * every mounted filesystem on the guest is thawed. + * + * Returns 0 on success, -1 otherwise. + */ +int +virDomainFSThaw(virDomainPtr dom, +const char **disks, +unsigned int ndisks, +unsigned int flags) +{ +VIR_DOMAIN_DEBUG(dom, "disks=%p, ndisks=%d, flags=%x", + disks, ndisks, flags); + +virResetLastError(); + +virCheckDomainReturn(dom, -1); +virCheckReadOnlyGoto(dom->conn->flags, error); +if (ndisks) +vir
[libvirt] [PATCH v5 2/5] remote: Implement virDomainFSFreeze and virDomainFSThaw
New rules are added in fixup_name in gendispatch.pl to keep the name FSFreeze and FSThaw. This adds a new ACL permission 'fs_freeze', which is also applied to VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE flag. Signed-off-by: Tomoki Sekiyama --- src/access/viraccessperm.c |2 +- src/access/viraccessperm.h |6 ++ src/remote/remote_driver.c |2 ++ src/remote/remote_protocol.x | 30 -- src/remote_protocol-structs | 18 ++ src/rpc/gendispatch.pl |2 ++ 6 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c index d517c66..462f46c 100644 --- a/src/access/viraccessperm.c +++ b/src/access/viraccessperm.c @@ -39,7 +39,7 @@ VIR_ENUM_IMPL(virAccessPermDomain, "start", "stop", "reset", "save", "delete", "migrate", "snapshot", "suspend", "hibernate", "core_dump", "pm_control", - "init_control", "inject_nmi", "send_input", "send_signal", "fs_trim", + "init_control", "inject_nmi", "send_input", "send_signal", "fs_trim", "fs_freeze", "block_read", "block_write", "mem_read", "open_graphics", "open_device", "screenshot", "open_namespace"); diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h index 6d14f05..ac48d70 100644 --- a/src/access/viraccessperm.h +++ b/src/access/viraccessperm.h @@ -242,6 +242,12 @@ typedef enum { */ VIR_ACCESS_PERM_DOMAIN_FS_TRIM, /* Issue TRIM to guest filesystems */ +/** + * @desc: Freeze and thaw domain filesystems + * @message: Freezing and thawing domain filesystems require authorization + */ +VIR_ACCESS_PERM_DOMAIN_FS_FREEZE,/* Freeze and thaw guest filesystems */ + /* Peeking at guest */ /** diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index ed7dde6..8b39a90 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -7800,6 +7800,8 @@ static virDriver remote_driver = { .domainMigrateFinish3Params = remoteDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = remoteDomainMigrateConfirm3Params, /* 1.1.0 */ .connectGetCPUModelNames = remoteConnectGetCPUModelNames, /* 1.1.3 */ +.domainFSFreeze = remoteDomainFSFreeze, /* 1.2.4 */ +.domainFSThaw = remoteDomainFSThaw, /* 1.2.4 */ }; static virNetworkDriver network_driver = { diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 6c445cc..13fa69b 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -235,6 +235,9 @@ const REMOTE_DOMAIN_JOB_STATS_MAX = 64; /* Upper limit on number of CPU models */ const REMOTE_CONNECT_CPU_MODELS_MAX = 8192; +/* Upper limit on number of disks to frozen */ +const REMOTE_DOMAIN_FSFREEZE_DISKS_MAX = 256; + /* UUID. VIR_UUID_BUFLEN definition comes from libvirt.h */ typedef opaque remote_uuid[VIR_UUID_BUFLEN]; @@ -2959,6 +2962,17 @@ struct remote_network_event_lifecycle_msg { int detail; }; +struct remote_domain_fsfreeze_args { +remote_nonnull_domain dom; +remote_nonnull_string disks; /* (const char **) */ +unsigned int flags; +}; + +struct remote_domain_fsthaw_args { +remote_nonnull_domain dom; +remote_nonnull_string disks; /* (const char **) */ +unsigned int flags; +}; /*- Protocol. -*/ @@ -4289,7 +4303,7 @@ enum remote_procedure { /** * @generate: both * @acl: domain:snapshot - * @acl: domain:write:VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE + * @acl: domain:fs_freeze:VIR_DOMAIN_SNAPSHOT_CREATE_QUIESCE */ REMOTE_PROC_DOMAIN_SNAPSHOT_CREATE_XML = 185, @@ -5275,5 +5289,17 @@ enum remote_procedure { * @generate: both * @acl: domain:core_dump */ -REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334 +REMOTE_PROC_DOMAIN_CORE_DUMP_WITH_FORMAT = 334, + +/** + * @generate: both + * @acl: domain:fs_freeze + */ +REMOTE_PROC_DOMAIN_FSFREEZE = 335, + +/** + * @generate: both + * @acl: domain:fs_freeze + */ +REMOTE_PROC_DOMAIN_FSTHAW = 336 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 456d0da..439ff87 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -2427,6 +2427,22 @@ struct remote_network_event_lifecycle_msg { intevent; intdetail; }; +struct remote_domain_fsfreeze_args { +remote_nonnull_domain dom; +struct { +u_int disks_len; +remote_nonnull_string * disks_val; +} disks; +u_int flags; +}; +struct remote_domain_fsthaw_args { +remote_nonnull_domain dom; +struct { +u_int disks_len; +remote_nonnull_string * disks_val; +} disks; +
[libvirt] [PATCH v5 0/5] Expose FSFreeze/FSThaw within the guest as API
Hello, This is patchset v5 to add FSFreeze/FSThaw API for custom disk snapshotting. Changes to v4: * add disks and ndisks parameter to specify disks to be frozen/thawed * make fsfreeze requests nestable * change api version to 1.2.4 (v4: https://www.redhat.com/archives/libvir-list/2014-March/msg01674.html ) === Description === Currently FSFreeze and FSThaw are supported by qemu guest agent and they are used internally in snapshot-create command with --quiesce option. However, when users want to utilize the native snapshot feature of storage devices (such as LVM over iSCSI, enterprise storage appliances, etc.), they need to issue fsfreeze command separately from libvirt-driven snapshots. (OpenStack cinder provides these storages' snapshot feature, but it cannot quiesce the guest filesystems automatically for now.) Although virDomainQemuGuestAgent() API could be used for this purpose, it is only for debugging and is not supported officially. This patchset adds virDomainFSFreeze()/virDomainFSThaw() APIs and virsh domfsfreeze/domfsthaw commands to enable the users to freeze and thaw domain's filesystems cleanly. The APIs take disks and ndisks parameters, which is a list of disk names to be frozen/thawed. If the option is not provided, every mounted filesystem is frozen/thawed. The fsfreeze can be nestable. When fsfreeze requests to a disk are issued multiple times, it is not thawed until the fsthaw requests are issued as many times as the freeze requests. Currently, qemu driver doesn't support disks parameter because the guest agent doesn't have means to specify disks to be frozen/thawed. Hence, it just counts depth of fsfreeze per domain, not per disk, so far. The APIs have flags option currently unsupported for future extension. --- Tomoki Sekiyama (5): Introduce virDomainFSFreeze() and virDomainFSThaw() public API remote: Implement virDomainFSFreeze and virDomainFSThaw qemu: Track domain quiesced status and make fsfreeze/thaw nestable qemu: Implement virDomainFSFreeze and virDomainFSThaw virsh: Expose new virDomainFSFreeze and virDomainFSThaw API include/libvirt/libvirt.h.in | 10 +++ src/access/viraccessperm.c |2 - src/access/viraccessperm.h |6 ++ src/driver.h | 14 src/libvirt.c| 92 src/libvirt_public.syms |6 ++ src/qemu/qemu_domain.c |6 ++ src/qemu/qemu_domain.h |2 + src/qemu/qemu_driver.c | 159 ++ src/remote/remote_driver.c |2 + src/remote/remote_protocol.x | 30 +++- src/remote_protocol-structs | 18 + src/rpc/gendispatch.pl |2 + tools/virsh-domain.c | 128 ++ tools/virsh.pod | 23 ++ 15 files changed, 483 insertions(+), 17 deletions(-) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5 4/5] qemu: Implement virDomainFSFreeze and virDomainFSThaw
Use qemuDomainSnapshotFSFreeze() and qemuDomainSnapshotFSFThaw() which are already implemented for snapshot quiescing. So far, disks and ndisks parameters are unsupported, because the guest agent doesn't have means to specify disks to be frozen or thawed. Signed-off-by: Tomoki Sekiyama --- src/qemu/qemu_driver.c | 94 1 file changed, 94 insertions(+) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2bb5fa9..81ff473 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16700,6 +16700,98 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, } +static int +qemuDomainFSFreeze(virDomainPtr dom, + const char **disks, + unsigned int ndisks, + unsigned int flags) +{ +virQEMUDriverPtr driver = dom->conn->privateData; +virDomainObjPtr vm; +int ret = -1; + +virCheckFlags(0, -1); + +if (disks || ndisks) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Specifying disks is not supported for now")); +return -1; +} + +if (!(vm = qemuDomObjFromDomain(dom))) +goto cleanup; + +if (virDomainFSFreezeEnsureACL(dom->conn, vm->def) < 0) +goto cleanup; + +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); +goto endjob; +} + +ret = qemuDomainSnapshotFSFreeze(driver, vm); + + endjob: +if (!qemuDomainObjEndJob(driver, vm)) +vm = NULL; + + cleanup: +if (vm) +virObjectUnlock(vm); +return ret; +} + + +static int +qemuDomainFSThaw(virDomainPtr dom, + const char **disks, + unsigned int ndisks, + unsigned int flags) +{ +virQEMUDriverPtr driver = dom->conn->privateData; +virDomainObjPtr vm; +int ret = -1; + +virCheckFlags(0, -1); + +if (disks || ndisks) { +virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("Specifying disks is not supported for now")); +return -1; +} + +if (!(vm = qemuDomObjFromDomain(dom))) +goto cleanup; + +if (virDomainFSThawEnsureACL(dom->conn, vm->def) < 0) +goto cleanup; + +if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0) +goto cleanup; + +if (!virDomainObjIsActive(vm)) { +virReportError(VIR_ERR_OPERATION_INVALID, + "%s", _("domain is not running")); +goto endjob; +} + +ret = qemuDomainSnapshotFSThaw(driver, vm, true); + + endjob: +if (!qemuDomainObjEndJob(driver, vm)) +vm = NULL; + + cleanup: +if (vm) +virObjectUnlock(vm); +return ret; +} + + static virDriver qemuDriver = { .no = VIR_DRV_QEMU, .name = QEMU_DRIVER_NAME, @@ -16890,6 +16982,8 @@ static virDriver qemuDriver = { .domainMigrateFinish3Params = qemuDomainMigrateFinish3Params, /* 1.1.0 */ .domainMigrateConfirm3Params = qemuDomainMigrateConfirm3Params, /* 1.1.0 */ .connectGetCPUModelNames = qemuConnectGetCPUModelNames, /* 1.1.3 */ +.domainFSFreeze = qemuDomainFSFreeze, /* 1.2.4 */ +.domainFSThaw = qemuDomainFSThaw, /* 1.2.4 */ }; -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 0/2] bhyve: add console support through nmdm device
On 30.03.2014 15:19, Roman Bogorodskiy wrote: Changes from v2: - Add unittest - Minor: new labels style and target for 1.2.4 Changes from v1: Switch over from implicit slave device path, i.e. guessing from that the slave is '/dev/nmdm0B', to explicit master and slave device specification using . Roman Bogorodskiy (2): bhyve: add console support through nmdm device bhyve: add xml2argv tests for console src/bhyve/bhyve_command.c | 34 + src/bhyve/bhyve_driver.c | 45 + src/conf/domain_audit.c| 1 + src/conf/domain_conf.c | 59 +- src/conf/domain_conf.h | 5 ++ src/qemu/qemu_command.c| 1 + src/qemu/qemu_monitor_json.c | 1 + tests/bhyvexml2argvdata/bhyvexml2argv-console.args | 4 ++ tests/bhyvexml2argvdata/bhyvexml2argv-console.xml | 23 + tests/bhyvexml2argvdata/bhyvexml2argv-serial.args | 4 ++ tests/bhyvexml2argvdata/bhyvexml2argv-serial.xml | 23 + tests/bhyvexml2argvtest.c | 2 + 12 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-console.xml create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial.args create mode 100644 tests/bhyvexml2argvdata/bhyvexml2argv-serial.xml ACK to both patches. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] Add V6LOCAL nwfilter parameter.
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. This is part of what information is needed to correctly filter guest IPv6 traffic. Since this changes with the MAC address, it is significantly easier if libvirt populates it (rather then requiring the user to enter it). As an example, an interface with a MAC address of "12:34:45:46:23:41" would have a V6LOCAL value of "fe80::1034:45ff:fe46:2341" --- docs/formatnwfilter.html.in| 11 --- src/conf/nwfilter_params.h |1 + src/nwfilter/nwfilter_gentech_driver.c | 25 + 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/docs/formatnwfilter.html.in b/docs/formatnwfilter.html.in index 45b97f7..4f10884 100644 --- a/docs/formatnwfilter.html.in +++ b/docs/formatnwfilter.html.in @@ -239,9 +239,9 @@ Usage of variables in filters - Two variables names have so far been reserved for usage by the - network traffic filtering subsystem: MAC and - IP. + Three variables names have so far been reserved for usage by the + network traffic filtering subsystem: MAC, + IP, and V6LOCAL (since 1.2.4). MAC is the MAC address of the network interface. A filtering rule that references this variable @@ -251,6 +251,11 @@ parameter similar to the IP parameter above, it is discouraged since libvirt knows what MAC address an interface will be using. + V6LOCAL is the computed IPv6 link-local address. + This is based on the MAC address of the interface. As an example, + a MAC address of 12:34:45:46:23:41 would result in a + link local address of fe80::1034:45ff:fe46:2341. + The parameter IP represents the IP address that the operating system inside the virtual machine is expected to use on the given interface. The IP parameter diff --git a/src/conf/nwfilter_params.h b/src/conf/nwfilter_params.h index 5e9777b..f61250f 100644 --- a/src/conf/nwfilter_params.h +++ b/src/conf/nwfilter_params.h @@ -98,6 +98,7 @@ bool virNWFilterHashTableEqual(virNWFilterHashTablePtr a, # define NWFILTER_VARNAME_IP "IP" # define NWFILTER_VARNAME_MAC "MAC" +# define NWFILTER_VARNAME_V6LOCAL "V6LOCAL" # define NWFILTER_VARNAME_CTRL_IP_LEARNING "CTRL_IP_LEARNING" # define NWFILTER_VARNAME_DHCPSERVER "DHCPSERVER" diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c index 1ce5e70..c58df1c 100644 --- a/src/nwfilter/nwfilter_gentech_driver.c +++ b/src/nwfilter/nwfilter_gentech_driver.c @@ -45,6 +45,7 @@ VIR_LOG_INIT("nwfilter.nwfilter_gentech_driver"); #define NWFILTER_STD_VAR_MAC NWFILTER_VARNAME_MAC #define NWFILTER_STD_VAR_IP NWFILTER_VARNAME_IP +#define NWFILTER_STD_VAR_V6LOCAL NWFILTER_VARNAME_V6LOCAL #define NWFILTER_DFLT_LEARN "any" @@ -163,6 +164,30 @@ 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::%02x%02x:%02xff:fe%02x:%02x%02x", parsedMac.addr[0], + parsedMac.addr[1], parsedMac.addr[2], parsedMac.addr[3], + parsedMac.addr[4], parsedMac.addr[5]); + +val = virNWFilterVarValueCreateSimpleCopyValue(euiMacAddr); +if (!val) +return -1; + +if (virHashAddEntry(table->hashTable, +NWFILTER_STD_VAR_V6LOCAL, +val) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, + "%s", _("Could not add variable 'V6LOCAL' " + "to hashmap")); +return -1; +} +} } if (ipaddr) { -- 1.7.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] libxl: Set disk format for empty cdrom device
On 27.03.2014 17:55, Stefan Bader wrote: The XML config for a CDROM device can be without a source path, indicating that there is no media present. Without this change the libxl driver fails to start a guest in that case because the libxl library checks for the LIBXL_DISK_FORMAT_EMPTY format type and tries to stat the NULL pointer that gets passed on. libxl: error: libxl_device.c:265:libxl__device_disk_set_backend: Disk vdev=hdc failed to stat: (null): Bad address Signed-off-by: Stefan Bader --- src/libxl/libxl_conf.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index de6f7ce..b8de72a 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -827,6 +827,9 @@ libxlMakeDisk(virDomainDiskDefPtr l_disk, libxl_device_disk *x_disk) x_disk->removable = 1; x_disk->readwrite = !l_disk->readonly; x_disk->is_cdrom = l_disk->device == VIR_DOMAIN_DISK_DEVICE_CDROM ? 1 : 0; +/* An empty CDROM must have the empty format, otherwise libxl fails. */ +if (x_disk->is_cdrom && !x_disk->pdev_path) +x_disk->format = LIBXL_DISK_FORMAT_EMPTY; if (l_disk->transient) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("libxenlight does not support transient disks")); ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] libxl: Implement basic video device selection
On 27.03.2014 17:55, Stefan Bader wrote: 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 --- src/libxl/libxl_conf.c | 85 1 file changed, 85 insertions(+) diff --git a/src/libxl/libxl_conf.c b/src/libxl/libxl_conf.c index b8de72a..fdd2726 100644 --- a/src/libxl/libxl_conf.c +++ b/src/libxl/libxl_conf.c @@ -1298,6 +1298,82 @@ libxlMakeCapabilities(libxl_ctx *ctx) return NULL; } +static void +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; +VIR_STRDUP(b_info->u.hvm.vnc.listen, vfb0->vnc.listen); +VIR_STRDUP(b_info->u.hvm.vnc.passwd, vfb0->vnc.passwd); +b_info->u.hvm.sdl = vfb0->sdl; +VIR_STRDUP(b_info->u.hvm.sdl.display, vfb0->sdl.display); +VIR_STRDUP(b_info->u.hvm.sdl.xauthority, vfb0->sdl.xauthority); +VIR_STRDUP(b_info->u.hvm.keymap, vfb0->keymap); You need to check for the return value of VIR_STRDUP(). Moreover, the indentation seems off. Looking forward to v2. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] libxl: Use id from virDomainObj inside the driver
On 27.03.2014 17:55, Stefan Bader wrote: There is a domain id in the virDomain structure as well as in the virDomainObj structure. While the former can become stale the latter is kept up to date. So it is safer to always (virDomainObjPtr)->def->id internally. This will fix issues seen when managing Xen guests through libvirt from virt-manager (not being able to get domain info after define or reboot). This was caused both though libxlDomainGetInfo() only but there were a lot of places that might potentially cause issues, too. Signed-off-by: Stefan Bader --- src/libxl/libxl_driver.c | 75 +++--- 1 file changed, 38 insertions(+), 37 deletions(-) Oh, you've sent the patch again. This time as a part of bigger set. Anyway, my ACK stands. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] beginner project idea: enum scrubbing [was: Fwd: Start contributing with Libvirt (Finding a mentor or helpful tips)]
It's a good suggestion to start. I will do that, Eric. =) -- *Julio Cesar Faracco* -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] cpu: Add documentation for CPU driver APIs
On 26.03.2014 16:08, Jiri Denemark wrote: Signed-off-by: Jiri Denemark --- src/cpu/cpu.c | 202 ++ 1 file changed, 202 insertions(+) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] libxl: Use id from virDomainObj inside the driver
On 25.03.2014 18:39, Stefan Bader wrote: There is a domain id in the virDomain structure as well as in the virDomainObj structure. While the former can become stale the latter is kept up to date. So it is safer to always (virDomainObjPtr)->def->id internally. This will fix issues seen when managing Xen guests through libvirt from virt-manager (not being able to get domain info after define or reboot). This was caused both though libxlDomainGetInfo() only but there were a lot of places that might potentially cause issues, too. Signed-off-by: Stefan Bader --- src/libxl/libxl_driver.c | 75 +++--- 1 file changed, 38 insertions(+), 37 deletions(-) ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] phyp: fix logic error on volume creation
On 02.04.2014 01:39, Eric Blake wrote: The phyp code claims that it wants a non-zero value, but actually enforces a capacity of zero. It has been this way since commit ebc46fe in June 2010. Bummer that it has my name as the committer - I guess I should have been much more stubborn about not blindly taking someone else's 1600-line patch. * src/phyp/phyp_driver.c (phypStorageVolCreateXML): Use correct logic. Signed-off-by: Eric Blake --- The fact that this bug has gone unnoticed for years makes me wonder if we are better off just removing the phyp driver from our code base, since it is obvious it is not getting much testing. I'm also waiting for a review on this, because although I _think_ the code wanted a non-zero capacity, I don't know enough about phyp and the "viosvrcmd -c 'mklv -lv'" command line; maybe the comments are wrong and it always wanted 0 capacity instead (which is the only thing that would get past the pre-patch code check). C'mon. Who's really creating volumes via libvirt? I mean, the fact that users don't create volumes via libvirt doesn't mean they are not using phyp driver. I probably should not say this aloud, but I used volume creation only when starting with libvirt. I used to play with virt-manager to get sense of libvirt's capabilities. I admit that things are different now sice we have domains and storage pools tied together, but not for me as I'm already used to doing it the other way. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add redirdevs to ABI stability check
On 02.04.2014 14:03, Ján Tomko wrote: Check the bus, type of the source device (tcp vs. spicevmc) and the device address visible in the guest. https://bugzilla.redhat.com/show_bug.cgi?id=1035128 --- src/conf/domain_conf.c | 50 ++ 1 file changed, 50 insertions(+) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 0af5be7..9f1c020 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13693,6 +13693,42 @@ virDomainHubDefCheckABIStability(virDomainHubDefPtr src, return true; } + +static bool +virDomainRedirdevDefCheckABIStability(virDomainRedirdevDefPtr src, + virDomainRedirdevDefPtr dst) +{ +if (src->bus != dst->bus) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target redirected device bus %s does not match " + "source %s"), + virDomainRedirdevBusTypeToString(dst->bus), + virDomainRedirdevBusTypeToString(src->bus)); +return false; +} + +switch ((enum virDomainRedirdevBus) src->bus) { +case VIR_DOMAIN_REDIRDEV_BUS_USB: +if (src->source.chr.type != dst->source.chr.type) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target redirected device source type %s does " + "not match source device source type %s"), + virDomainChrTypeToString(dst->source.chr.type), + virDomainChrTypeToString(src->source.chr.type)); +return false; +} +break; +case VIR_DOMAIN_REDIRDEV_BUS_LAST: +break; +} + +if (!virDomainDeviceInfoCheckABIStability(&src->info, &dst->info)) +return false; + +return true; +} + + static bool virDomainRedirFilterDefCheckABIStability(virDomainRedirFilterDefPtr src, virDomainRedirFilterDefPtr dst) @@ -14133,6 +14169,20 @@ virDomainDefCheckABIStability(virDomainDefPtr src, if (!virDomainHubDefCheckABIStability(src->hubs[i], dst->hubs[i])) goto error; +if (src->nredirdevs != dst->nredirdevs) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain redirected devices count %zu " + "does not match source %zu"), + dst->nconsoles, src->nconsoles); +goto error; +} + +for (i = 0; i < src->nredirdevs; i++) { +if (!virDomainRedirdevDefCheckABIStability(src->redirdevs[i], + dst->redirdevs[i])) +goto error; +} + if ((!src->redirfilter && dst->redirfilter) || (src->redirfilter && !dst->redirfilter)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Define CPUINFO_FILE_LEN and fix maxlen of cpuinfo file for powerpc
On 04/03/2014 12:43 AM, Olivia Yin wrote: > This patch define CPUINFO_FILE_LEN as 2KB which is enough for most > architectures. > > For the 24 cores PowerPC machines, the file of /proc/cpuinfo will be larger > than 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 > > So this patch increases the maxlen of cpuinfo for PowerPC architecture. > > Signed-off-by: Olivia Yin > --- > 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..33c4bc6 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, 2 * CPUINFO_FILE_LEN, &outbuf) < 0) { Uggh. Having to multiply the constant means you defined the constant wrong. Just make CPUINFO_FILE_LEN big enough. Even as large as 1M, for ALL uses, is not a burden. > +++ b/src/util/virsysinfo.h > @@ -102,4 +102,6 @@ bool virSysinfoIsEqual(virSysinfoDefPtr src, > > VIR_ENUM_DECL(virSysinfo) > > +#define CPUINFO_FILE_LEN 2048/*2KB limit for normal cpuinfo file*/ > + > #endif /* __VIR_SYSINFOS_H__ */ > -- 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 v4] Introduce --without-pm-utils to get rid of pm-is-supported dependency
This uses the dbus api of systemd to check the power management capabilities of the node. --- Diff with v3: * Added unit tests vir virSystemdCan* helpers * Make the default for with-pm-utils depending on dbus and systemd detection * Changed the implementation of the helpers to return true for 'yes' and 'challenge' responses, we shouldn't get a 'no' given libvirtd runs as privileged user... but who knows. configure.ac | 24 +++ libvirt.spec.in | 9 + src/libvirt_private.syms | 3 ++ src/util/virnodesuspend.c | 75 ++ src/util/virsystemd.c | 59 +++ src/util/virsystemd.h | 6 +++ tests/virsystemdmock.c| 22 ++ tests/virsystemdtest.c| 100 +- 8 files changed, 281 insertions(+), 17 deletions(-) diff --git a/configure.ac b/configure.ac index 73efffa..34e5ec2 100644 --- a/configure.ac +++ b/configure.ac @@ -563,6 +563,10 @@ AC_ARG_WITH([chrdev-lock-files], [location for UUCP style lock files for character devices (use auto for default paths on some platforms) @<:@default=auto@:>@])]) m4_divert_text([DEFAULTS], [with_chrdev_lock_files=auto]) +AC_ARG_WITH([pm-utils], + [AS_HELP_STRING([--with-pm-utils], +[use pm-utils for power management @<:@default=yes@:>@])]) +m4_divert_text([DEFAULTS], [with_pm_utils=check]) dnl dnl in case someone want to build static binaries @@ -1621,6 +1625,25 @@ fi AM_CONDITIONAL([WITH_PHYP],[test "$with_phyp" = "yes"]) +dnl +dnl Should we build with pm-utils support? +dnl +set -x +if test "$with_pm_utils" = "check"; then +with_pm_utils=yes +if test "$with_dbus" = "yes"; then +if test "$init_systemd" = "yes"; then +with_pm_utils=no +fi +fi +fi + +if test "$with_pm_utils" = "yes"; then +AC_DEFINE_UNQUOTED([WITH_PM_UTILS], 1, [whether to use pm-utils]) +fi +AM_CONDITIONAL([WITH_PM_UTILS], [test "$with_pm_utils" = "yes"]) +set +x + dnl virsh libraries VIRSH_LIBS="$VIRSH_LIBS $READLINE_LIBS" AC_SUBST([VIRSH_LIBS]) @@ -2845,6 +2868,7 @@ AC_MSG_NOTICE([ rbd: $LIBRBD_LIBS]) else AC_MSG_NOTICE([ rbd: no]) fi +AC_MSG_NOTICE([pm-utils: $with_pm_utils]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Test suite]) diff --git a/libvirt.spec.in b/libvirt.spec.in index eab9b23..5c20955 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -132,6 +132,7 @@ %define with_libssh2 0%{!?_without_libssh2:0} %define with_wireshark 0%{!?_without_wireshark:0} %define with_systemd_daemon 0%{!?_without_systemd_daemon:0} +%define with_pm_utils 1 # Non-server/HV driver defaults which are always enabled %define with_sasl 0%{!?_without_sasl:1} @@ -182,6 +183,7 @@ %if 0%{?fedora} >= 17 || 0%{?rhel} >= 7 %define with_systemd 1 %define with_systemd_daemon 1 +%define with_pm_utils 0 %endif # Fedora 18 / RHEL-7 are first where firewalld support is enabled @@ -1138,8 +1140,10 @@ Requires: nc Requires: gettext # Needed by virt-pki-validate script. Requires: gnutls-utils +%if %{with_pm_utils} # Needed for probing the power management features of the host. Requires: pm-utils +%{endif} %if %{with_sasl} Requires: cyrus-sasl # Not technically required, but makes 'out-of-box' config @@ -1395,6 +1399,10 @@ driver %define _without_systemd_daemon --without-systemd-daemon %endif +%if ! %{with_pm_utils} +%define _without_pm_utils --without-pm-utils +%endif + %define when %(date +"%%F-%%T") %define where %(hostname) %define who %{?packager}%{!?packager:Unknown} @@ -1471,6 +1479,7 @@ rm -f po/stamp-po %{?_with_firewalld} \ %{?_without_wireshark} \ %{?_without_systemd_daemon} \ + %{?_without_pm_utils} \ %{with_packager} \ %{with_packager_version} \ --with-qemu-user=%{qemu_user} \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 38fbf63..ce51bdf 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1879,6 +1879,9 @@ virSysinfoSetup; # util/virsystemd.h +virSystemdCanHibernate; +virSystemdCanHybridSleep; +virSystemdCanSuspend; virSystemdCreateMachine; virSystemdMakeMachineName; virSystemdMakeScopeName; diff --git a/src/util/virnodesuspend.c b/src/util/virnodesuspend.c index 8088931..b1eddff 100644 --- a/src/util/virnodesuspend.c +++ b/src/util/virnodesuspend.c @@ -22,6 +22,7 @@ #include #include "virnodesuspend.h" +# include "virsystemd.h" #include "vircommand.h" #include "virthread.h" #include "datatypes.h" @@ -245,23 +246,9 @@ int nodeSuspendForDuration(unsigned int target, return ret; } - -/** - * virNodeSuspendSupportsTarget: - * @target: The power management target to check whether it is supported - * by the host. Values could be: - * VIR_NODE_SUSPEND_TARGET_MEM - * VIR_NODE_SUSPEND_TARGET_DISK - * VIR_NODE_SUSPEND_T
Re: [libvirt] [PATCH] Use the force flag for mkfs -t xfs
On 03.04.2014 12:23, Ján Tomko wrote: Without this, building an XFS pool on a formatted partition fails with --overwrite. https://bugzilla.redhat.com/show_bug.cgi?id=927172 --- src/storage/storage_backend_fs.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index e0244ba..1d85871 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -665,11 +665,13 @@ virStorageBackendExecuteMKFS(const char *device, int ret = 0; virCommandPtr cmd = NULL; -cmd = virCommandNewArgList(MKFS, - "-t", - format, - device, - NULL); +cmd = virCommandNewArgList(MKFS, "-t", format, NULL); + +/* use the force, otherwise mkfs.xfs won't overwrite existing fs */ +if (STREQ(format, "xfs")) +virCommandAddArg(cmd, "-f"); + +virCommandAddArg(cmd, device); if (virCommandRun(cmd, NULL) < 0) { virReportSystemError(errno, ACK Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Use the force flag for mkfs -t xfs
Without this, building an XFS pool on a formatted partition fails with --overwrite. https://bugzilla.redhat.com/show_bug.cgi?id=927172 --- src/storage/storage_backend_fs.c | 12 +++- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index e0244ba..1d85871 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -665,11 +665,13 @@ virStorageBackendExecuteMKFS(const char *device, int ret = 0; virCommandPtr cmd = NULL; -cmd = virCommandNewArgList(MKFS, - "-t", - format, - device, - NULL); +cmd = virCommandNewArgList(MKFS, "-t", format, NULL); + +/* use the force, otherwise mkfs.xfs won't overwrite existing fs */ +if (STREQ(format, "xfs")) +virCommandAddArg(cmd, "-f"); + +virCommandAddArg(cmd, device); if (virCommandRun(cmd, NULL) < 0) { virReportSystemError(errno, -- 1.8.3.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Make 'exit' action same as 'quit'
On 02.04.2014 10:43, liyang wrote: From: Li Yang For now 'virsh quit' action like this: [root@localhost /]# virsh quit [root@localhost /]# And 'virsh exit' action: [root@localhost /]# virsh exit [root@localhost /]# There is a small difference('/n') between them. According to manual said: quit, exit quit this interactive terminal And in the code they all called cmdQuit func, They should get same actions. Signed-off-by: Li Yang --- tools/virsh.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/tools/virsh.c b/tools/virsh.c index 02835d9..da7fedd 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -1852,7 +1852,8 @@ vshCommandRun(vshControl *ctl, const vshCmd *cmd) if (!ret && disconnected != 0) vshReconnect(ctl); -if (STREQ(cmd->def->name, "quit"))/* hack ... */ +if (STREQ(cmd->def->name, "quit") || +STREQ(cmd->def->name, "exit"))/* hack ... */ return ret; if (enable_timing) { ACKed and pushed. Congratulations on your second libvirt contribution! Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: make sure agent returns error when required data are missing
On Thu, Apr 03, 2014 at 09:32:02AM +0200, Peter Krempa wrote: On 04/03/14 07:58, Martin Kletzander wrote: Commit 5b3492fa aimed to fix this and caught one error but exposed another one. When agent command is being executed and the thread waiting for the reply is woken up by an event (e.g. EOF in case of shutdown), the command finishes with no data (rxObject == NULL), but no error is reported, since this might be desired by the caller (e.g. suspend through agent). However, in other situations, when the data are required (e.g. getting vCPUs), we proceed to getting desired data out of the reply, but none of the virJSON*() functions works well with NULLs. I chose the way of a new parameter for qemuAgentCommand() function that specifies whether reply is required and behaves according to that. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1058149 Signed-off-by: Martin Kletzander --- Notes: This issue probably exists since qemu-ga is supported in libvirt, so this (along with 5b3492fa and e9d09fe1) might be worth back-porting to some maintenance branches, I just haven't gone through them to see which ones are applicable. src/qemu/qemu_agent.c | 19 ++- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 92573bd..4082331 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1080,6 +1080,7 @@ static int qemuAgentCommand(qemuAgentPtr mon, virJSONValuePtr cmd, virJSONValuePtr *reply, + bool needReply, int seconds) { Seeing that the "needReply" parameter is true in most cases, I'd rename this function and add one not requiring the new parameter for normal usage and use the renamed in the few cases that actually expect the VM to quit. I don't think it's worth adding this much of unnecessary code just to eliminate one argument in 4 out of 7 call, especially when all the functions are private and even static, therefore not much of a cleanup from the readability POV. But this is just a cosmetic issue. Patches are welcome, though ;) ACK Thanks, pushed Peter Martin signature.asc Description: Digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] bhyve: create capabilities submodule
On 02.04.2014 10:30, Wojciech Macek wrote: Adding one minor change to copyrights header... - Move all capabilities functions to separate file - Add initCPU --- src/Makefile.am| 2 + src/bhyve/bhyve_capabilities.c | 105 ++ +++ src/bhyve/bhyve_capabilities.h | 30 src/bhyve/bhyve_driver.c | 56 +- 4 files changed, 171 insertions(+), 22 deletions(-) create mode 100644 src/bhyve/bhyve_capabilities.c create mode 100644 src/bhyve/bhyve_capabilities.h This is not the best way to send patch as reply. I see you've used git send-email to send the previous patches. If you want to send a v2 just to a specific one (say 2/3), you should use git send-email too; just specify in-reply-to and adjust subject prefix: PATCHv2: Moreover, sending patches other way than git send-email is tricky as you need to make sure the lines are not wrapped (yes, it happened in this case too). Can you resend the v2? It's okay to send v2 just to a single patch. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 3/3] qemu: Implement virDomain{Get,Set}Time
One caveat though, qemu-ga is expecting time and returning time in nanoseconds. With all the buffering and propagation delay, the time is already wrong once it gets to the qemu-ga, but there's nothing we can do about it. Signed-off-by: Michal Privoznik --- src/qemu/qemu_agent.c | 99 src/qemu/qemu_agent.h | 8 src/qemu/qemu_driver.c | 109 + 3 files changed, 216 insertions(+) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 92573bd..4a8fa87 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -1645,3 +1645,102 @@ qemuAgentUpdateCPUInfo(unsigned int nvcpus, return 0; } + + +int +qemuAgentGetTime(qemuAgentPtr mon, + long long *seconds, + unsigned int *nseconds) +{ +int ret = -1; +unsigned long long json_time; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +cmd = qemuAgentMakeCommand("guest-get-time", + NULL); +if (!cmd) +return ret; + +if (qemuAgentCommand(mon, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) +goto cleanup; + +if (!reply || qemuAgentCheckError(cmd, reply) < 0) +goto cleanup; + +if (virJSONValueObjectGetNumberUlong(reply, "return", &json_time) < 0) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed return value")); +goto cleanup; +} + +/* guest agent returns time in nanoseconds, + * we need it in seconds here */ +*seconds = json_time / 10LL; +*nseconds = json_time % 10LL; +ret = 0; + + cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} + + +/** + * qemuAgentSetTime: + * @setTime: time to set + * @sync: let guest agent to read domain's RTC (@setTime is ignored) + */ +int +qemuAgentSetTime(qemuAgentPtr mon, +long long seconds, +unsigned int nseconds, +bool sync) +{ +int ret = -1; +virJSONValuePtr cmd; +virJSONValuePtr reply = NULL; + +if (sync) { +cmd = qemuAgentMakeCommand("guest-set-time", NULL); +} else { +/* guest agent expect time with nanosecond granularity. + * Impressing. */ +long long json_time; + +/* Check if we overflow. For some reason qemu doesn't handle unsigned + * long long on the monitor well as it silently truncates numbers to + * signed long long. Therefore we must check overflow against LLONG_MAX + * not ULLONG_MAX. */ +if (seconds > LLONG_MAX / 10LL) { +virReportError(VIR_ERR_INVALID_ARG, + _("Time '%lld' is too big for guest agent"), + seconds); +return ret; +} + +json_time = seconds * 10LL; +json_time += nseconds; +cmd = qemuAgentMakeCommand("guest-set-time", + "I:time", json_time, + NULL); +} + +if (!cmd) +return ret; + +if (qemuAgentCommand(mon, cmd, &reply, + VIR_DOMAIN_QEMU_AGENT_COMMAND_BLOCK) < 0) +goto cleanup; + +if (!reply || qemuAgentCheckError(cmd, reply) < 0) +goto cleanup; + +ret = 0; + cleanup: +virJSONValueFree(cmd); +virJSONValueFree(reply); +return ret; +} diff --git a/src/qemu/qemu_agent.h b/src/qemu/qemu_agent.h index 5fbacdb..399b586 100644 --- a/src/qemu/qemu_agent.h +++ b/src/qemu/qemu_agent.h @@ -97,4 +97,12 @@ int qemuAgentSetVCPUs(qemuAgentPtr mon, qemuAgentCPUInfoPtr cpus, size_t ncpus); int qemuAgentUpdateCPUInfo(unsigned int nvcpus, qemuAgentCPUInfoPtr cpuinfo, int ncpuinfo); + +int qemuAgentGetTime(qemuAgentPtr mon, + long long *seconds, + unsigned int *nseconds); +int qemuAgentSetTime(qemuAgentPtr mon, + long long seconds, + unsigned int nseconds, + bool sync); #endif /* __QEMU_AGENT_H__ */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 1d08951..e661f28 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -16640,6 +16640,113 @@ qemuConnectGetCPUModelNames(virConnectPtr conn, return cpuGetModels(arch, models); } +static int +qemuDomainGetTime(virDomainPtr dom, + long long *seconds, + unsigned int *nseconds, + unsigned int flags) +{ +virQEMUDriverPtr driver = dom->conn->privateData; +virDomainObjPtr vm = NULL; +qemuDomainObjPrivatePtr priv; +int ret = -1; +int rv; + +virCheckFlags(0, ret); + +if (!(vm = qemuDomObjFromDomain(dom))) +return ret; + +if (virDomainGetTimeEnsureACL(dom->conn, vm->def) < 0) +goto cleanup; + +
[libvirt] [PATCHv3 0/3] Time setting and getting in qemu guests
Hopefully the last round. Michal Privoznik (3): Introduce virDomain{Get,Set}Time APIs virsh: Expose virDomain{Get,Set}Time qemu: Implement virDomain{Get,Set}Time daemon/remote.c | 37 include/libvirt/libvirt.h.in | 14 + src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 7 ++- src/driver.h | 14 + src/libvirt.c| 94 ++ src/libvirt_public.syms | 5 ++ src/qemu/qemu_agent.c| 99 src/qemu/qemu_agent.h| 8 +++ src/qemu/qemu_driver.c | 109 +++ src/remote/remote_driver.c | 35 src/remote/remote_protocol.x | 31 +- src/remote_protocol-structs | 16 ++ tools/virsh-domain-monitor.c | 132 +++ tools/virsh.pod | 16 ++ 15 files changed, 616 insertions(+), 3 deletions(-) -- 1.9.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCHv3 2/3] virsh: Expose virDomain{Get,Set}Time
These APIs are exposed under new virsh command 'domtime' which both gets and sets (not at the same time of course :)). Signed-off-by: Michal Privoznik --- tools/virsh-domain-monitor.c | 132 +++ tools/virsh.pod | 16 ++ 2 files changed, 148 insertions(+) diff --git a/tools/virsh-domain-monitor.c b/tools/virsh-domain-monitor.c index 18d551a..31fa8de 100644 --- a/tools/virsh-domain-monitor.c +++ b/tools/virsh-domain-monitor.c @@ -1356,6 +1356,132 @@ cmdDomstate(vshControl *ctl, const vshCmd *cmd) } /* + * "domtime" command + */ +static const vshCmdInfo info_domtime[] = { +{.name = "help", + .data = N_("domain time") +}, +{.name = "desc", + .data = N_("Gets or sets a domain time") +}, +{.name = NULL} +}; + +static const vshCmdOptDef opts_domtime[] = { +{.name = "domain", + .type = VSH_OT_DATA, + .flags = VSH_OFLAG_REQ, + .help = N_("domain name, id or uuid") +}, +{.name = "now", + .type = VSH_OT_BOOL, + .help = N_("set current host time") +}, +{.name = "pretty", + .type = VSH_OT_BOOL, + .help = N_("print domain's time in human readable form") +}, +{.name = "sync", + .type = VSH_OT_BOOL, + .help = N_("instead of setting given time, synchronize from domain's RTC"), +}, +{.name = "time", + .type = VSH_OT_INT, + .help = N_("time to set") +}, +{.name = NULL} +}; + +static bool +cmdDomTime(vshControl *ctl, const vshCmd *cmd) +{ +virDomainPtr dom; +bool ret = false; +bool now = vshCommandOptBool(cmd, "now"); +bool pretty = vshCommandOptBool(cmd, "pretty"); +bool sync = vshCommandOptBool(cmd, "sync"); +long long seconds = 0; +unsigned int nseconds = 0; +unsigned int flags = 0; +bool doSet = false; +int rv; + +if (!(dom = vshCommandOptDomain(ctl, cmd, NULL))) +return false; + +rv = vshCommandOptLongLong(cmd, "time", &seconds); + +if (rv < 0) { +/* invalid integer format */ +vshError(ctl, "%s", + _("Unable to parse integer parameter to --time.")); +goto cleanup; +} else if (rv > 0) { +/* --time is used, so set time instead of get time. + * However, --time and --now are mutually exclusive. */ +if (now) { +vshError(ctl, _("--time and --now are mutually exclusive")); +goto cleanup; +} + +/* Neither is --time and --sync */ +if (sync) { +vshError(ctl, _("--time and --sync are mutually exclusive")); +goto cleanup; + +} +doSet = true; +} + +if (sync && now) { +vshError(ctl, _("--sync and --now are mutually exclusive")); +goto cleanup; +} + +/* --now or --sync means setting */ +doSet |= now | sync; + +if (doSet) { +if (now && ((seconds = time(NULL)) == (time_t) -1)) { +vshError(ctl, _("Unable to get current time")); +goto cleanup; +} + +if (sync) +flags |= VIR_DOMAIN_TIME_SYNC; + +if (virDomainSetTime(dom, seconds, nseconds, flags) < 0) +goto cleanup; + +} else { +if (virDomainGetTime(dom, &seconds, &nseconds, flags) < 0) +goto cleanup; + +if (pretty) { +char timestr[100]; +time_t cur_time = seconds; +struct tm time_info; + +if (!gmtime_r(&cur_time, &time_info)) { +vshError(ctl, _("Unable to format time")); +goto cleanup; +} +strftime(timestr, sizeof(timestr), "%Y-%m-%d %H:%M:%S", &time_info); + +vshPrint(ctl, _("Time: %s"), timestr); +} else { +vshPrint(ctl, _("Time: %lld"), seconds); +} +} + +ret = true; + cleanup: +virDomainFree(dom); +return ret; +} + +/* * "list" command */ static const vshCmdInfo info_list[] = { @@ -1911,6 +2037,12 @@ const vshCmdDef domMonitoringCmds[] = { .info = info_domstate, .flags = 0 }, +{.name = "domtime", + .handler = cmdDomTime, + .opts = opts_domtime, + .info = info_domtime, + .flags = 0 +}, {.name = "list", .handler = cmdList, .opts = opts_list, diff --git a/tools/virsh.pod b/tools/virsh.pod index 98d891a..d2de8a6 100644 --- a/tools/virsh.pod +++ b/tools/virsh.pod @@ -994,6 +994,22 @@ Returns state of an interface to VMM used to control a domain. For states other than "ok" or "error" the command also prints number of seconds elapsed since the control interface entered its current state. +=item B I { [I<--now>] [I<--pretty>] [I<--sync>] +[I<--time> B] } + +Gets or sets the domain's system time. When run without any arguments +(but I), the current domain's system time is printed out. The +I<--pretty> modifier can be used to print the time in more human +readable form. When I<--time> B is specified, the domain's time is +not get
[libvirt] [PATCHv3 1/3] Introduce virDomain{Get,Set}Time APIs
These APIs allow users to get or set time in a domain, which may come handy if the domain has been resumed just recently and NTP is not configured or hasn't kicked in yet and the guest is running something time critical. In addition, NTP may refuse to re-set the clock if the skew is too big. In addition, new ACL attribute is introduced 'set_time'. Signed-off-by: Michal Privoznik --- daemon/remote.c | 37 + include/libvirt/libvirt.h.in | 14 +++ src/access/viraccessperm.c | 2 +- src/access/viraccessperm.h | 7 +++- src/driver.h | 14 +++ src/libvirt.c| 94 src/libvirt_public.syms | 5 +++ src/remote/remote_driver.c | 35 + src/remote/remote_protocol.x | 31 ++- src/remote_protocol-structs | 16 10 files changed, 252 insertions(+), 3 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 8476961..34c96c9 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6077,6 +6077,43 @@ qemuDispatchConnectDomainMonitorEventDeregister(virNetServerPtr server ATTRIBUTE return rv; } +static int +remoteDispatchDomainGetTime(virNetServerPtr server ATTRIBUTE_UNUSED, +virNetServerClientPtr client, +virNetMessagePtr msg ATTRIBUTE_UNUSED, +virNetMessageErrorPtr rerr, +remote_domain_get_time_args *args, +remote_domain_get_time_ret *ret) +{ +int rv = -1; +virDomainPtr dom = NULL; +long long seconds; +unsigned int nseconds; +struct daemonClientPrivate *priv = +virNetServerClientGetPrivateData(client); + +if (!priv->conn) { +virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); +goto cleanup; +} + +if (!(dom = get_nonnull_domain(priv->conn, args->dom))) +goto cleanup; + +if (virDomainGetTime(dom, &seconds, &nseconds, args->flags) < 0) +goto cleanup; + +ret->seconds = seconds; +ret->nseconds = nseconds; +rv = 0; + + cleanup: +if (rv < 0) +virNetMessageSaveError(rerr); +if (dom) +virDomainFree(dom); +return rv; +} /*- Helpers. -*/ diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 930b7e8..988f00d 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5277,6 +5277,20 @@ int virDomainFSTrim(virDomainPtr dom, unsigned long long minimum, unsigned int flags); +int virDomainGetTime(virDomainPtr dom, + long long *seconds, + unsigned int *nseconds, + unsigned int flags); + +typedef enum { +VIR_DOMAIN_TIME_SYNC = (1 << 0), /* Re-sync domain time from domain's RTC */ +} virDomainSetTimeFlags; + +int virDomainSetTime(virDomainPtr dom, + long long seconds, + unsigned int nseconds, + unsigned int flags); + /** * virSchedParameterType: * diff --git a/src/access/viraccessperm.c b/src/access/viraccessperm.c index d517c66..bbcb6c1 100644 --- a/src/access/viraccessperm.c +++ b/src/access/viraccessperm.c @@ -42,7 +42,7 @@ VIR_ENUM_IMPL(virAccessPermDomain, "init_control", "inject_nmi", "send_input", "send_signal", "fs_trim", "block_read", "block_write", "mem_read", "open_graphics", "open_device", "screenshot", - "open_namespace"); + "open_namespace", "set_time"); VIR_ENUM_IMPL(virAccessPermInterface, VIR_ACCESS_PERM_INTERFACE_LAST, diff --git a/src/access/viraccessperm.h b/src/access/viraccessperm.h index 6d14f05..6fa0f01 100644 --- a/src/access/viraccessperm.h +++ b/src/access/viraccessperm.h @@ -282,13 +282,18 @@ typedef enum { */ VIR_ACCESS_PERM_DOMAIN_SCREENSHOT,/* Trigger a screen shot */ - /** * @desc: Open domain namespace * @message: Opening domain namespaces requires authorization */ VIR_ACCESS_PERM_DOMAIN_OPEN_NAMESPACE, +/** + * @desc: Write domain time + * @message: Setting the domain time requires authorization + */ +VIR_ACCESS_PERM_DOMAIN_SET_TIME, + VIR_ACCESS_PERM_DOMAIN_LAST, } virAccessPermDomain; diff --git a/src/driver.h b/src/driver.h index e66fc7a..474d045 100644 --- a/src/driver.h +++ b/src/driver.h @@ -1084,6 +1084,18 @@ typedef int unsigned int flags); typedef int +(*virDrvDomainGetTime)(virDomainPtr dom, + long long *seconds, + unsigned int *nseconds, + unsigned int flags); + +typedef int +(*virDrvDomainSetTime)(virDomainPtr dom, + long long seconds, + unsigned int nseconds, + unsigned int flags); + +typedef int (*
Re: [libvirt] [PATCH] qemu: make sure agent returns error when required data are missing
On 04/03/14 07:58, Martin Kletzander wrote: > Commit 5b3492fa aimed to fix this and caught one error but exposed > another one. When agent command is being executed and the thread > waiting for the reply is woken up by an event (e.g. EOF in case of > shutdown), the command finishes with no data (rxObject == NULL), but > no error is reported, since this might be desired by the caller > (e.g. suspend through agent). However, in other situations, when the > data are required (e.g. getting vCPUs), we proceed to getting desired > data out of the reply, but none of the virJSON*() functions works well > with NULLs. I chose the way of a new parameter for qemuAgentCommand() > function that specifies whether reply is required and behaves > according to that. > > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1058149 > > Signed-off-by: Martin Kletzander > --- > > Notes: > This issue probably exists since qemu-ga is supported in libvirt, so > this (along with 5b3492fa and e9d09fe1) might be worth back-porting to > some maintenance branches, I just haven't gone through them to see > which ones are applicable. > > src/qemu/qemu_agent.c | 19 ++- > 1 file changed, 10 insertions(+), 9 deletions(-) > > diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c > index 92573bd..4082331 100644 > --- a/src/qemu/qemu_agent.c > +++ b/src/qemu/qemu_agent.c > @@ -1080,6 +1080,7 @@ static int > qemuAgentCommand(qemuAgentPtr mon, > virJSONValuePtr cmd, > virJSONValuePtr *reply, > + bool needReply, > int seconds) > { Seeing that the "needReply" parameter is true in most cases, I'd rename this function and add one not requiring the new parameter for normal usage and use the renamed in the few cases that actually expect the VM to quit. But this is just a cosmetic issue. ACK Peter signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Include PCI address in the error in virDomainNetFindIdx
On 04/02/2014 01:53 PM, Laine Stump wrote: > On 04/01/2014 06:11 PM, Ján Tomko wrote: >> When looking up a net device by a MAC and PCI address, it is possible >> that we've got a match on the MAC address but failed to match the >> PCI address. >> >> In that case, outputting just the MAC address can be confusing. >> >> Partially resolves: >> https://bugzilla.redhat.com/show_bug.cgi?id=872028 >> --- >> src/conf/domain_conf.c | 17 ++--- >> 1 file changed, 14 insertions(+), 3 deletions(-) >> >> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c >> index 1624c7e..35defaf 100644 >> --- a/src/conf/domain_conf.c >> +++ b/src/conf/domain_conf.c >> @@ -10456,9 +10456,20 @@ virDomainNetFindIdx(virDomainDefPtr def, >> virDomainNetDefPtr net) >> } >> } >> if (matchidx < 0) { >> -virReportError(VIR_ERR_OPERATION_FAILED, >> - _("no device matching mac address %s found"), >> - virMacAddrFormat(&net->mac, mac)); >> +if (PCIAddrSpecified) { >> +virReportError(VIR_ERR_OPERATION_FAILED, >> + _("no device matching mac address %s found on " >> + "%.4x:%.2x:%.2x.%.1x"), >> + virMacAddrFormat(&net->mac, mac), >> + net->info.addr.pci.domain, >> + net->info.addr.pci.bus, >> + net->info.addr.pci.slot, >> + net->info.addr.pci.function); > > We really should make a virPCIAddrFormat() function and start using it... > > That's a separate issue though. ACK to this patch. > Thanks, I've pushed the series. Jan signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list