[libvirt] [PATCH] util: Error out if the numa nodeset is out of range
Instead of a silent warning, it's better to error out if the numa nodeset is out of range. Just like for numa node larger than NUMA_NUM_NODES. --- src/util/virnuma.c | 12 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index bace06f..902ed43 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -89,7 +89,6 @@ virNumaSetupMemoryPolicy(virNumaTuneDef numatune, int ret = -1; int i = 0; int maxnode = 0; -bool warned = false; virBitmapPtr tmp_nodemask = NULL; if (numatune.memory.placement_mode == @@ -113,20 +112,17 @@ virNumaSetupMemoryPolicy(virNumaTuneDef numatune, } maxnode = numa_max_node() + 1; + /* Convert nodemask to NUMA bitmask. */ nodemask_zero(mask); i = -1; while ((i = virBitmapNextSetBit(tmp_nodemask, i)) = 0) { -if (i NUMA_NUM_NODES) { +if (i maxnode || i NUMA_NUM_NODES) { virReportError(VIR_ERR_INTERNAL_ERROR, - _(Host cannot support NUMA node %d), i); + _(Nodeset is out of range, host cannot support + NUMA node bigger than %d), i); return -1; } -if (i maxnode !warned) { -VIR_WARN(nodeset is out of range, there is only %d NUMA - nodes on host, maxnode); -warned = true; -} nodemask_set(mask, i); } -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1][RESEND] ppc64 cpu features
On Mon, Apr 22, 2013 at 10:39:46 +0800, Li Zhang wrote: On 2013年04月19日 22:04, Jiri Denemark wrote: On Thu, Mar 14, 2013 at 14:54:21 +0800, Li Zhang wrote: From: Li Zhang zhlci...@linux.vnet.ibm.com This patch adds a CPU feature powernv identifying IBM Power processor that supports native hypervisor e.g. KVM. This can be used by virtualization management software to determine the CPU capabilities. PVR doesn't indicate whether it a host or a guest CPU. So, we use /proc/cpuinfo to get the platform information (PowerNV) and use that to set the powernv flag. Signed-off-by: Dipankar Sarma dipan...@in.ibm.com Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- src/cpu/cpu_map.xml|9 ++ src/cpu/cpu_powerpc.c | 349 ++-- src/cpu/cpu_ppc_data.h |4 + src/util/sysinfo.c |2 +- 4 files changed, 294 insertions(+), 70 deletions(-) Looks like this patch was not even rebased since it was written back in time 1.0.1 was released. Anyway, I realized I did not push my changes to powerpc driver so I did that. And I also started rewriting this patch on top of it since this patch is rather huge and seems to mix lots of things. Also PowerPC CPUs seem to be quite different from x86 CPUs so enhancing powerpc driver by copypasting code from x86 driver is not the best way :-) It just makes powerpc driver unnecessarily complicated. We hope it matches x86 driver, so most code are from x86. There is no CPUID on powerpc, but we also need feature such as powernv. And CPUID is what makes x86 driver so complicated. That's why I think it makes more sense to implement the right functionality from scratch rather than copypasting x86 driver :-) So what is this powernv feature used for? It won't show up in capabilities XML as it is included in all powerpc CPU models. That also This feature means that powerpc can support KVM. Other platform, for example, pseries doesn't support KVM. So we need this feature. Currently, we haven't introduced this feature in capabilities XML yet. means, users don't need to explicitly enable it when configuring guest CPU. Thus it could only make sense to allow users to disable this feature for a given guest. However, I don't see powernv string anywhere in QEMU source code and thus it cannot really be used in any way in guest CPU definition. This feature is from sysinfo on host, not from QEMU. QEMU haven't included this feature yet. I am not sure whether it is necessary to introduce this to QEMU. I think I understand what powernv feature is but I don't get what it is (planned to be) used for in libvirt. And I'd like to understand that :-) The CPU modeling stuff in libvirt is used for *guest* CPU configuration, i.e., users can configure what CPU model and features the guest should see. So unless you want it to enable/disable something like nested virt, I don't see a lot of sense in introducing such feature. (I may be wrong of course, that's why I'm asking.) Capabilities XML provides some details about host CPU but that's mostly to give an idea about what guest CPU configuration the host could be able to provide. If you want to add powernv feature just because you need to distinguish if the host supports KVM or not, there's much better way... In guest section of capabilities XML, KVM support is indicated by domain type='kvm' element. And that's what existing apps already use to detect KVM presence/absence. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] network: Don't remove transient network if creating of config file fails
On the off-chance that creation of persistent configuration file would fail when defining a network that is already started as transient, the code would remove the transient data structure and thus the network. This patch changes the code so that in such case, the network is again marked as transient and left behind. --- src/network/bridge_driver.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 27dd230..64c71af 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3160,8 +3160,13 @@ static virNetworkPtr networkDefine(virConnectPtr conn, const char *xml) { freeDef = false; if (virNetworkSaveConfig(driver-networkConfigDir, def) 0) { -virNetworkRemoveInactive(driver-networks, network); -network = NULL; +if (!virNetworkObjIsActive(network)) { +virNetworkRemoveInactive(driver-networks, network); +network = NULL; +goto cleanup; +} +network-persistent = 0; +virNetworkDefFree(network-newDef); goto cleanup; } -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/1][RESEND] ppc64 cpu features
On 2013年04月22日 17:10, Jiri Denemark wrote: On Mon, Apr 22, 2013 at 10:39:46 +0800, Li Zhang wrote: On 2013年04月19日 22:04, Jiri Denemark wrote: On Thu, Mar 14, 2013 at 14:54:21 +0800, Li Zhang wrote: From: Li Zhang zhlci...@linux.vnet.ibm.com This patch adds a CPU feature powernv identifying IBM Power processor that supports native hypervisor e.g. KVM. This can be used by virtualization management software to determine the CPU capabilities. PVR doesn't indicate whether it a host or a guest CPU. So, we use /proc/cpuinfo to get the platform information (PowerNV) and use that to set the powernv flag. Signed-off-by: Dipankar Sarma dipan...@in.ibm.com Signed-off-by: Li Zhang zhlci...@linux.vnet.ibm.com --- src/cpu/cpu_map.xml|9 ++ src/cpu/cpu_powerpc.c | 349 ++-- src/cpu/cpu_ppc_data.h |4 + src/util/sysinfo.c |2 +- 4 files changed, 294 insertions(+), 70 deletions(-) Looks like this patch was not even rebased since it was written back in time 1.0.1 was released. Anyway, I realized I did not push my changes to powerpc driver so I did that. And I also started rewriting this patch on top of it since this patch is rather huge and seems to mix lots of things. Also PowerPC CPUs seem to be quite different from x86 CPUs so enhancing powerpc driver by copypasting code from x86 driver is not the best way :-) It just makes powerpc driver unnecessarily complicated. We hope it matches x86 driver, so most code are from x86. There is no CPUID on powerpc, but we also need feature such as powernv. And CPUID is what makes x86 driver so complicated. That's why I think it makes more sense to implement the right functionality from scratch rather than copypasting x86 driver :-) OK, you are right. So what is this powernv feature used for? It won't show up in capabilities XML as it is included in all powerpc CPU models. That also This feature means that powerpc can support KVM. Other platform, for example, pseries doesn't support KVM. So we need this feature. Currently, we haven't introduced this feature in capabilities XML yet. means, users don't need to explicitly enable it when configuring guest CPU. Thus it could only make sense to allow users to disable this feature for a given guest. However, I don't see powernv string anywhere in QEMU source code and thus it cannot really be used in any way in guest CPU definition. This feature is from sysinfo on host, not from QEMU. QEMU haven't included this feature yet. I am not sure whether it is necessary to introduce this to QEMU. I think I understand what powernv feature is but I don't get what it is (planned to be) used for in libvirt. And I'd like to understand that :-) The CPU modeling stuff in libvirt is used for *guest* CPU configuration, i.e., users can configure what CPU model and features the guest should see. So unless you want it to enable/disable something like nested virt, I don't see a lot of sense in introducing such feature. (I may be wrong of course, that's why I'm asking.) Capabilities XML provides some details about host CPU but that's mostly to give an idea about what guest CPU configuration the host could be able to provide. I see. This feature is read-only. Nested virt isn't considered yet. Actually, I haven't understood this completely. I think PowerPC doesn't support nest virt. I just thought that migration is only allowed with 'powernv'. Currently, only powernv platform can work with KVM. In the future, there may be more features of CPU. If you want to add powernv feature just because you need to distinguish if the host supports KVM or not, there's much better way... In guest section of capabilities XML, KVM support is indicated by domain type='kvm' element. And that's what existing apps already use to detect KVM presence/absence. As my understanding, there is still some difference from 'kvm' capability. 'powernv' is only considered as one CPU feature of PPC64. If other PPC platforms support KVM in the future, this feature can be used to identify whether migration can be executed . :) Jirka -- Li Zhang IBM China Linux Technology Centre -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Fix the wrong expression
Wrong use of the parentheses causes rc always having a boolean value, either 1 or 0, and thus we can't get the detailed error message when it fails: Before (I only have 1 node): % virsh numatune f18 --nodeset 12 error: Unable to change numa parameters error: unable to set numa tunable: Unknown error -1 After: virsh numatune f18 --nodeset 12 error: Unable to change numa parameters error: unable to set numa tunable: Invalid argument --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index cee5557..0de7ffd 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -7470,7 +7470,7 @@ qemuDomainSetNumaParameters(virDomainPtr dom, continue; } -if ((rc = virCgroupSetCpusetMems(priv-cgroup, nodeset_str) != 0)) { +if ((rc = virCgroupSetCpusetMems(priv-cgroup, nodeset_str)) != 0) { virReportSystemError(-rc, %s, _(unable to set numa tunable)); virBitmapFree(nodeset); -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix the wrong expression
On 04/22/13 13:40, Osier Yang wrote: Wrong use of the parentheses causes rc always having a boolean value, either 1 or 0, and thus we can't get the detailed error message when it fails: Before (I only have 1 node): % virsh numatune f18 --nodeset 12 error: Unable to change numa parameters error: unable to set numa tunable: Unknown error -1 After: virsh numatune f18 --nodeset 12 error: Unable to change numa parameters error: unable to set numa tunable: Invalid argument --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix the wrong expression
On 22/04/13 18:56, Peter Krempa wrote: On 04/22/13 13:40, Osier Yang wrote: Wrong use of the parentheses causes rc always having a boolean value, either 1 or 0, and thus we can't get the detailed error message when it fails: Before (I only have 1 node): % virsh numatune f18 --nodeset 12 error: Unable to change numa parameters error: unable to set numa tunable: Unknown error -1 After: virsh numatune f18 --nodeset 12 error: Unable to change numa parameters error: unable to set numa tunable: Invalid argument --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) ACK. Peter Thanks, pushed. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Use camelCase for XML attribute numQueues
On Thu, Apr 18, 2013 at 08:27:56AM -0600, Eric Blake wrote: On 04/18/2013 03:02 AM, Martin Kletzander wrote: In commit d4bf0a9, we used num_queues for an attribute in the XML, but the consensus is that we use camelCase for that. Since there was no release yet (the above commit describes as v1.0.4-65-gd4bf0a9), we still have time to change it. You may want to wait for DV's opinion on naming, but I wasn't aware that we had a consensus on camelCase. In fact, '_' is currently winning, although it's hard to say whether that is limited to older interfaces. Hum, I don't think I ever suggested such rules at the XML level. We have that convention for code, as eric pointed out it is not as clear for XML (and unfortunately we can't hamonize at this point without quite a bit of churn - we would have to support the old syntax forever anyway). I don't have a strong opinion on _ vs. camelcase for markup, I guess the exact same arguments can be raised pros/cons on visibility and existing use :-\ Seems Eric is rather adverse to _ which was looking like the winner, I dislike '-' myself as it is used for comments, and very uncommon in identifiers in XML (well it's barred from being in an identifier in most language). If Eric really can't stand '_' then maybe the simplest is really to start standardizing on camelCase, Daniel -- Daniel Veillard | Open Source and Standards, Red Hat veill...@redhat.com | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3] qemu: Implement CPUs check against machine type's cpu-max
ping? On 04/17/2013 04:15 PM, Michal Novotny wrote: Implement check whether (maximum) vCPUs doesn't exceed machine type's cpu-max settings. On older versions of QEMU the check is disabled. Signed-off-by: Michal Novotny minov...@redhat.com --- src/qemu/qemu_capabilities.c | 38 +- src/qemu/qemu_capabilities.h | 3 ++- src/qemu/qemu_monitor.h | 1 + src/qemu/qemu_monitor_json.c | 7 +++ src/qemu/qemu_process.c | 27 +++ 5 files changed, 74 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d10c8aa..bbfe85f 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -243,6 +243,7 @@ struct _virQEMUCaps { size_t nmachineTypes; char **machineTypes; char **machineAliases; +int *machineMaxCpus; }; struct _virQEMUCapsCache { @@ -322,6 +323,7 @@ virQEMUCapsSetDefaultMachine(virQEMUCapsPtr qemuCaps, { char *name = qemuCaps-machineTypes[defIdx]; char *alias = qemuCaps-machineAliases[defIdx]; +int cpu_max = qemuCaps-machineMaxCpus[defIdx]; memmove(qemuCaps-machineTypes + 1, qemuCaps-machineTypes, @@ -329,8 +331,12 @@ virQEMUCapsSetDefaultMachine(virQEMUCapsPtr qemuCaps, memmove(qemuCaps-machineAliases + 1, qemuCaps-machineAliases, sizeof(qemuCaps-machineAliases[0]) * defIdx); +memmove(qemuCaps-machineMaxCpus + 1, +qemuCaps-machineMaxCpus, +sizeof(qemuCaps-machineMaxCpus[0]) * defIdx); qemuCaps-machineTypes[0] = name; qemuCaps-machineAliases[0] = alias; +qemuCaps-machineMaxCpus[0] = cpu_max; } /* Format is: @@ -377,7 +383,8 @@ virQEMUCapsParseMachineTypesStr(const char *output, } if (VIR_REALLOC_N(qemuCaps-machineTypes, qemuCaps-nmachineTypes + 1) 0 || -VIR_REALLOC_N(qemuCaps-machineAliases, qemuCaps-nmachineTypes + 1) 0) { +VIR_REALLOC_N(qemuCaps-machineAliases, qemuCaps-nmachineTypes + 1) 0 || +VIR_REALLOC_N(qemuCaps-machineMaxCpus, qemuCaps-nmachineTypes + 1) 0) { VIR_FREE(name); VIR_FREE(canonical); goto no_memory; @@ -390,6 +397,8 @@ virQEMUCapsParseMachineTypesStr(const char *output, qemuCaps-machineTypes[qemuCaps-nmachineTypes-1] = name; qemuCaps-machineAliases[qemuCaps-nmachineTypes-1] = NULL; } +/* Value 0 means unknown as it's not exposed by QEMU binary */ +qemuCaps-machineMaxCpus[qemuCaps-nmachineTypes-1] = 0; } while ((p = next)); @@ -1718,6 +1727,8 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) goto no_memory; if (VIR_ALLOC_N(ret-machineAliases, qemuCaps-nmachineTypes) 0) goto no_memory; +if (VIR_ALLOC_N(ret-machineMaxCpus, qemuCaps-nmachineTypes) 0) +goto no_memory; ret-nmachineTypes = qemuCaps-nmachineTypes; for (i = 0 ; i qemuCaps-nmachineTypes ; i++) { if (!(ret-machineTypes[i] = strdup(qemuCaps-machineTypes[i]))) @@ -1725,6 +1736,7 @@ virQEMUCapsPtr virQEMUCapsNewCopy(virQEMUCapsPtr qemuCaps) if (qemuCaps-machineAliases[i] !(ret-machineAliases[i] = strdup(qemuCaps-machineAliases[i]))) goto no_memory; +ret-machineMaxCpus[i] = qemuCaps-machineMaxCpus[i]; } return ret; @@ -1923,6 +1935,25 @@ const char *virQEMUCapsGetCanonicalMachine(virQEMUCapsPtr qemuCaps, } +int virQEMUCapsGetMachineMaxCpus(virQEMUCapsPtr qemuCaps, + const char *name) +{ +size_t i; + +if (!name) +return 0; + +for (i = 0 ; i qemuCaps-nmachineTypes ; i++) { +if (!qemuCaps-machineMaxCpus[i]) +continue; +if (STREQ(qemuCaps-machineTypes[i], name)) +return qemuCaps-machineMaxCpus[i]; +} + +return 0; +} + + static int virQEMUCapsProbeQMPCommands(virQEMUCapsPtr qemuCaps, qemuMonitorPtr mon) @@ -2073,6 +2104,10 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, virReportOOMError(); goto cleanup; } +if (VIR_ALLOC_N(qemuCaps-machineMaxCpus, nmachines) 0) { +virReportOOMError(); +goto cleanup; +} for (i = 0 ; i nmachines ; i++) { if (machines[i]-alias) { @@ -2087,6 +2122,7 @@ virQEMUCapsProbeQMPMachineTypes(virQEMUCapsPtr qemuCaps, } if (machines[i]-isDefault) defIdx = i; +qemuCaps-machineMaxCpus[i] = machines[i]-cpu_max; } qemuCaps-nmachineTypes = nmachines; diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4e76799..bd97e8d 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -223,7 +223,8 @@ size_t
Re: [libvirt] [PATCH 1/2] virnetdevtap: add virNetDevTapGetName
On Sat, Apr 20, 2013 at 11:11:24AM +0200, Paolo Bonzini wrote: This will be used on a tap file descriptor returned by the bridge helper to populate the target element, because the helper does not provide the interface name. Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- src/libvirt_private.syms | 1 + src/util/virnetdevtap.c | 33 + src/util/virnetdevtap.h | 3 +++ 3 files changed, 37 insertions(+) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] qemu: launch bridge helper from libvirtd
On Sat, Apr 20, 2013 at 11:11:25AM +0200, Paolo Bonzini wrote: source type='bridge' uses a helper application to do the necessary TUN/TAP setup to use an existing network bridge, thus letting unprivileged users use TUN/TAP interfaces. However, libvirt should be preventing QEMU from running any setuid programs at all, which would include this helper program. From a security POV, any setuid helper needs to be run by libvirtd itself, not QEMU. This is what this patch does. libvirt now invokes the setuid helper, gets the TAP fd and then passes it to QEMU in the normal manner. The path to the helper is specified in qemu.conf. As a small advantage, this adds a target dev='tap0'/ element to the XML of an active domain using interface type='bridge'. That's very good because it allows the network interfaces stats API to work Signed-off-by: Paolo Bonzini pbonz...@redhat.com --- src/qemu/qemu_command.c | 133 +++- src/qemu/qemu_command.h | 1 - src/qemu/qemu_hotplug.c | 25 +++-- 3 files changed, 106 insertions(+), 53 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Omitting braces with a single-line body
From: Harry Wei harryxi...@gmail.com After i read libvirt/HACKING file, i find we should Omit braces with a single-line body. So this patch fix this coding style problem for Sheepdog storage backend driver. Signed-off-by: Harry Wei harryxi...@gmail.com --- src/storage/storage_backend_sheepdog.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 35a3a04..15fa29c 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -97,12 +97,10 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, const char *address = localhost; int port = 7000; if (pool-def-source.nhost 0) { -if (pool-def-source.hosts[0].name != NULL) { +if (pool-def-source.hosts[0].name != NULL) address = pool-def-source.hosts[0].name; -} -if (pool-def-source.hosts[0].port) { +if (pool-def-source.hosts[0].port) port = pool-def-source.hosts[0].port; -} } virCommandAddArg(cmd, -a); virCommandAddArgFormat(cmd, %s, address); @@ -210,11 +208,10 @@ virStorageBackendSheepdogParseVdiList(virStorageVolDefPtr vol, continue; /* skip space */ -if (p + 2 next) { +if (p + 2 next) p += 2; -} else { +else return -1; -} /* skip name */ while (*p != '\0' *p != ' ') { -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Omitting braces with a single-line body
On 22/04/13 20:33, harryxi...@gmail.com wrote: From: Harry Wei harryxi...@gmail.com After i read libvirt/HACKING file, i find we should s/i/I/, Omit braces with a single-line body. So this patch fix this coding style problem for Sheepdog storage s/fix this/fixes the/, backend driver. The commit log can be more compact: Prefer no braces for single line bodies. Signed-off-by: Harry Wei harryxi...@gmail.com --- src/storage/storage_backend_sheepdog.c | 11 --- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/storage/storage_backend_sheepdog.c b/src/storage/storage_backend_sheepdog.c index 35a3a04..15fa29c 100644 --- a/src/storage/storage_backend_sheepdog.c +++ b/src/storage/storage_backend_sheepdog.c @@ -97,12 +97,10 @@ virStorageBackendSheepdogAddHostArg(virCommandPtr cmd, const char *address = localhost; int port = 7000; if (pool-def-source.nhost 0) { -if (pool-def-source.hosts[0].name != NULL) { +if (pool-def-source.hosts[0].name != NULL) address = pool-def-source.hosts[0].name; -} -if (pool-def-source.hosts[0].port) { +if (pool-def-source.hosts[0].port) port = pool-def-source.hosts[0].port; -} } virCommandAddArg(cmd, -a); virCommandAddArgFormat(cmd, %s, address); @@ -210,11 +208,10 @@ virStorageBackendSheepdogParseVdiList(virStorageVolDefPtr vol, continue; /* skip space */ -if (p + 2 next) { +if (p + 2 next) p += 2; -} else { +else return -1; -} /* skip name */ while (*p != '\0' *p != ' ') { ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Omitting braces with a single-line body
On Mon, Apr 22, 2013 at 9:13 PM, Osier Yang jy...@redhat.com wrote: On 22/04/13 20:33, harryxi...@gmail.com wrote: From: Harry Wei harryxi...@gmail.com After i read libvirt/HACKING file, i find we should s/i/I/, Actually, i wonder what 's/i/l/' means? Omit braces with a single-line body. So this patch fix this coding style problem for Sheepdog storage s/fix this/fixes the/, The same problem as above. backend driver. The commit log can be more compact: Prefer no braces for single line bodies. Yup, i think so ;-) [...] ACK Thanks ;-) -- Thanks Harry Wei -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] test: Add JSON test for query-tpm-types
Add a test case for query-tpm-models QMP command. Signed-off-by: Stefan Berger stef...@linux.vnet.ibm.com --- tests/qemumonitorjsontest.c | 55 1 file changed, 55 insertions(+) Index: libvirt/tests/qemumonitorjsontest.c === --- libvirt.orig/tests/qemumonitorjsontest.c +++ libvirt/tests/qemumonitorjsontest.c @@ -25,6 +25,7 @@ #include qemu/qemu_conf.h #include virthread.h #include virerror.h +#include virstring.h #define VIR_FROM_THIS VIR_FROM_NONE @@ -440,6 +441,59 @@ cleanup: static int +testQemuMonitorJSONGetTPMModels(const void *data) +{ +virDomainXMLOptionPtr xmlopt = (virDomainXMLOptionPtr)data; +qemuMonitorTestPtr test = qemuMonitorTestNew(true, xmlopt); +int ret = -1; +char **tpmmodels = NULL; +int ntpmmodels = 0; + +if (!test) +return -1; + +if (qemuMonitorTestAddItem(test, query-tpm-models, + { + \return\: [ + \passthrough\ + ] + }) 0) +goto cleanup; + +if ((ntpmmodels = qemuMonitorGetTPMModels(qemuMonitorTestGetMonitor(test), + tpmmodels)) 0) +goto cleanup; + +if (ntpmmodels != 1) { +virReportError(VIR_ERR_INTERNAL_ERROR, + ntpmmodels %d is not 1, ntpmmodels); +goto cleanup; +} + +#define CHECK(i, wantname) \ +do {\ +if (STRNEQ(tpmmodels[i], (wantname))) { \ + virReportError(VIR_ERR_INTERNAL_ERROR, \ + name %s is not %s, \ + tpmmodels[i], (wantname));\ +goto cleanup; \ + } \ +} while (0) + +CHECK(0, passthrough); + +#undef CHECK + +ret = 0; + +cleanup: +qemuMonitorTestFree(test); +virStringFreeList(tpmmodels); +return ret; +} + + +static int mymain(void) { int ret = 0; @@ -465,6 +519,7 @@ mymain(void) DO_TEST(GetMachines); DO_TEST(GetCPUDefinitions); DO_TEST(GetCommands); +DO_TEST(GetTPMModels); virObjectUnref(xmlopt); -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 0/5] Support NBD volumes with LXC containers
A second version of: https://www.redhat.com/archives/libvir-list/2013-March/msg00941.html Aside from the addressing the previous round of feedbac, this adds use of the new '--format' flag to qemu-nbd. This is introduced due to CVE-2013-1922. The upshot is that this code won't work with any qemu-nbd binary lacking the --format flag. This is intentionale. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 1/5] Add support for storage format in FS driver
From: Daniel P. Berrange berra...@redhat.com Extend the driver element in filesystem devices to allow a storage format to be set. The new attribute uses 'format' to reflect the storage format. This is different from the driver element in disk devices which use 'type' to reflect the storage format. This is because the 'type' attribute on filesystem devices is already used for the driver backend, for which the disk devices use the 'name' attribute. Arh. Anyway for disks we have driver name=qemu type=raw/ And for filesystems this change means we now have driver type=loop format=raw/ Signed-off-by: Daniel P. Berrange berra...@redhat.com --- docs/formatdomain.html.in | 24 ++ docs/schemas/domaincommon.rng | 73 ++- src/conf/domain_conf.c| 25 --- src/conf/domain_conf.h| 6 ++-- src/qemu/qemu_command.c | 3 +- 5 files changed, 102 insertions(+), 29 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 888c005..a707cc8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1876,6 +1876,13 @@ lt;target dir='/import/from/host'/gt; lt;readonly/gt; lt;/filesystemgt; +lt;filesystem type='file' accessmode='passthrough'gt; + lt;driver name='loop' type='raw'/gt; + lt;driver type='path' wrpolicy='immediate'/gt; + lt;source file='/export/to/guest.img'/gt; + lt;target dir='/import/from/host'/gt; + lt;readonly/gt; +lt;/filesystemgt; ... lt;/devicesgt; .../pre @@ -1967,6 +1974,23 @@ /dd + dtcodedriver/code/dt + dd +The optional driver element allows specifying further details +related to the hypervisor driver used to provide the filesystem. +span class=sinceSince 1.0.4/span +ul + li +If the hypervisor supports multiple backend drivers, then +the codetype/code attribute selects the primary +backend driver name, while the codeformat/code +attribute provides the format type. For example, LXC +supports a type of loop, with a format of raw. QEMU +supports a type of path or handle, but no formats. + /li +/ul + /dd + dtcodesource/code/dt dd The resource on the host that is being accessed in the guest. The diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..206757f 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -888,7 +888,7 @@ define name=diskspec interleave optional -ref name=driver/ +ref name=diskDriver/ /optional optional ref name='diskMirror'/ @@ -1270,7 +1270,7 @@ !-- Disk may use a special driver for access. -- - define name=driver + define name=diskDriver element name=driver choice group @@ -1314,13 +1314,13 @@ optional attribute name='type' choice - ref name='diskFormat'/ + ref name='storageFormat'/ valueaio/value !-- back-compat for 'raw' -- /choice /attribute /optional /define - define name='diskFormat' + define name='storageFormat' choice valueraw/value valuedir/value @@ -1502,6 +1502,9 @@ attribute name=type valuefile/value /attribute + optional +ref name=diskDriver/ + /optional interleave element name=source attribute name=file @@ -1515,6 +1518,9 @@ attribute name=type valueblock/value /attribute + optional +ref name=diskDriver/ + /optional interleave element name=source attribute name=dev @@ -1531,6 +1537,9 @@ valuemount/value /attribute /optional + optional +ref name=diskDriver/ + /optional interleave element name=source attribute name=dir @@ -1538,22 +1547,6 @@ /attribute empty/ /element -optional - element name=driver -attribute name=type - choice -valuepath/value -valuehandle/value - /choice -/attribute -optional - attribute name=wrpolicy -valueimmediate/value - /attribute -/optional -empty/ - /element -/optional /interleave /group group @@ -1562,6 +1555,9 @@ valuebind/value /attribute /optional + optional +ref name=diskDriver/ + /optional interleave
[libvirt] [PATCH v2 5/5] Support NBD backed disks/filesystems in LXC driver
From: Daniel P. Berrange berra...@redhat.com The LXC driver can already configure disk or filesystem devices to use the loop device. This extends it to also allow for use of the NBD device, to support non-raw formats. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/lxc/lxc_controller.c | 80 ++-- 1 file changed, 78 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index a91ea79..26bc960 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -389,6 +389,62 @@ static int virLXCControllerSetupLoopDeviceDisk(virDomainDiskDefPtr disk) } +static int virLXCControllerSetupNBDDeviceFS(virDomainFSDefPtr fs) +{ +char *dev; + +if (fs-format = VIR_STORAGE_FILE_NONE) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(An explicit disk format must be specified)); +return -1; +} + +if (virFileNBDDeviceAssociate(fs-src, + fs-format, + !!fs-readonly, + dev) 0) +return -1; + +/* + * We now change it into a block device type, so that + * the rest of container setup 'just works' + */ +fs-type = VIR_DOMAIN_DISK_TYPE_BLOCK; +VIR_FREE(fs-src); +fs-src = dev; + +return 0; +} + + +static int virLXCControllerSetupNBDDeviceDisk(virDomainDiskDefPtr disk) +{ +char *dev; + +if (disk-format = VIR_STORAGE_FILE_NONE) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(An explicit disk format must be specified)); +return -1; +} + +if (virFileNBDDeviceAssociate(disk-src, + disk-format, + !!disk-readonly, + dev) 0) +return -1; + +/* + * We now change it into a block device type, so that + * the rest of container setup 'just works' + */ +disk-type = VIR_DOMAIN_DISK_TYPE_BLOCK; +VIR_FREE(disk-src); +disk-src = dev; + +return 0; +} + + static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) { size_t i; @@ -421,6 +477,9 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) goto cleanup; } ctrl-loopDevFds[ctrl-nloopDevs - 1] = fd; +} else if (fs-fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_NBD) { +if (virLXCControllerSetupNBDDeviceFS(fs) 0) +goto cleanup; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(fs driver %s is not supported), @@ -435,8 +494,14 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) if (disk-type != VIR_DOMAIN_DISK_TYPE_FILE) continue; -if (!disk-driverName || -STREQ(disk-driverName, loop)) { +/* If no driverName is set, we prefer 'loop' for + * dealing with raw or undefined formats, otherwise + * we use 'nbd'. + */ +if (STREQ_NULLABLE(disk-driverName, loop) || +(!disk-driverName + (disk-format == VIR_STORAGE_FILE_RAW || + disk-format == VIR_STORAGE_FILE_NONE))) { if (disk-format != VIR_STORAGE_FILE_RAW disk-format != VIR_STORAGE_FILE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -460,6 +525,17 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) goto cleanup; } ctrl-loopDevFds[ctrl-nloopDevs - 1] = fd; +} else if (STREQ_NULLABLE(disk-driverName, nbd) || + !disk-driverName) { +if (disk-cachemode != VIR_DOMAIN_DISK_CACHE_DEFAULT +disk-cachemode != VIR_DOMAIN_DISK_CACHE_DISABLE) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(Disk cache mode %s is not supported), + virDomainDiskCacheTypeToString(disk-cachemode)); +goto cleanup; +} +if (virLXCControllerSetupNBDDeviceDisk(disk) 0) +goto cleanup; } else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _(disk driver %s is not supported), -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 2/5] Re-arrange code setting up ifs/disk loop devices for LXC
From: Daniel P. Berrange berra...@redhat.com The current code for setting up loop devices to LXC disks first does a switch() based on the disk format, then looks at the disk driver name. Reverse this so it first looks at the driver name, and then the disk format. This is more useful since the list of supported disk formats depends on what driver is used. The code for setting loop devices for LXC fs entries also needs to have the same logic added, now the XML schema supports this. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/lxc/lxc_controller.c | 76 +++- 1 file changed, 43 insertions(+), 33 deletions(-) diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c index 740b872..a91ea79 100644 --- a/src/lxc/lxc_controller.c +++ b/src/lxc/lxc_controller.c @@ -401,17 +401,31 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) if (fs-type != VIR_DOMAIN_FS_TYPE_FILE) continue; -fd = virLXCControllerSetupLoopDeviceFS(fs); -if (fd 0) -goto cleanup; +if (fs-fsdriver == VIR_DOMAIN_FS_DRIVER_TYPE_LOOP) { +if (fs-format != VIR_STORAGE_FILE_RAW +fs-format != VIR_STORAGE_FILE_NONE) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(fs format %s is not supported), + virStorageFileFormatTypeToString(fs-format)); +goto cleanup; +} -VIR_DEBUG(Saving loop fd %d, fd); -if (VIR_EXPAND_N(ctrl-loopDevFds, ctrl-nloopDevs, 1) 0) { -VIR_FORCE_CLOSE(fd); -virReportOOMError(); -goto cleanup; +fd = virLXCControllerSetupLoopDeviceFS(fs); +if (fd 0) +goto cleanup; + +VIR_DEBUG(Saving loop fd %d, fd); +if (VIR_EXPAND_N(ctrl-loopDevFds, ctrl-nloopDevs, 1) 0) { +VIR_FORCE_CLOSE(fd); +virReportOOMError(); +goto cleanup; +} +ctrl-loopDevFds[ctrl-nloopDevs - 1] = fd; +} else { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(fs driver %s is not supported), + virDomainFSDriverTypeTypeToString(fs-fsdriver)); } -ctrl-loopDevFds[ctrl-nloopDevs - 1] = fd; } for (i = 0 ; i ctrl-def-ndisks ; i++) { @@ -421,40 +435,36 @@ static int virLXCControllerSetupLoopDevices(virLXCControllerPtr ctrl) if (disk-type != VIR_DOMAIN_DISK_TYPE_FILE) continue; -switch (disk-format) { -/* We treat 'none' as meaning 'raw' since we - * don't want to go into the auto-probing - * business for security reasons - */ -case VIR_STORAGE_FILE_RAW: -case VIR_STORAGE_FILE_NONE: -if (disk-driverName -STRNEQ(disk-driverName, loop)) { +if (!disk-driverName || +STREQ(disk-driverName, loop)) { +if (disk-format != VIR_STORAGE_FILE_RAW +disk-format != VIR_STORAGE_FILE_NONE) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(disk driver %s is not supported), - disk-driverName); + _(disk format %s is not supported), + virStorageFileFormatTypeToString(disk-format)); goto cleanup; } +/* We treat 'none' as meaning 'raw' since we + * don't want to go into the auto-probing + * business for security reasons + */ fd = virLXCControllerSetupLoopDeviceDisk(disk); if (fd 0) goto cleanup; -break; -default: +VIR_DEBUG(Saving loop fd %d, fd); +if (VIR_EXPAND_N(ctrl-loopDevFds, ctrl-nloopDevs, 1) 0) { +VIR_FORCE_CLOSE(fd); +virReportOOMError(); +goto cleanup; +} +ctrl-loopDevFds[ctrl-nloopDevs - 1] = fd; +} else { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(disk format %s is not supported), - virStorageFileFormatTypeToString(disk-format)); -goto cleanup; -} - -VIR_DEBUG(Saving loop fd %d, fd); -if (VIR_EXPAND_N(ctrl-loopDevFds, ctrl-nloopDevs, 1) 0) { -VIR_FORCE_CLOSE(fd); -virReportOOMError(); -goto cleanup; + _(disk driver %s is not supported), + disk-driverName); } -ctrl-loopDevFds[ctrl-nloopDevs - 1] = fd; } VIR_DEBUG(Setup all loop devices); -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rng: tighten up domain controller schema
On 04/19/2013 11:25 AM, Laine Stump wrote: On 04/19/2013 04:32 AM, Osier Yang wrote: On 18/04/13 19:59, Laine Stump wrote: On 04/18/2013 07:27 AM, Osier Yang wrote: On 18/04/13 19:16, Laine Stump wrote: On 04/18/2013 05:41 AM, Martin Kletzander wrote: On 04/18/2013 11:05 AM, Osier Yang wrote: On 18/04/13 17:00, Martin Kletzander wrote: On 04/18/2013 10:54 AM, Osier Yang wrote: On 18/04/13 16:42, Martin Kletzander wrote: On 04/18/2013 06:36 AM, Laine Stump wrote: The rng schema for controller had been non-specific about which types of controllers allowed which models, and also allowed the num_queues attribute (since that hasn't been released yet, should we rename it to numQueues?) Since there's still time (the commit with that is v1.0.4-65-gd4bf0a9), I think we should rename it ASAP since we are using camelCase for all the attribute names. Apart from that, the RNG with this patch is precise according to the documentation, so ACK. I'll try to send the numQueues patch to see what others think. I guess you mean multiple queues support for virtio network? Regardless of which style we will use finally, FYI, num_queues is used for disk.. Personally I'm fine with either, because we already use both across. Yes, I meant the virtio-scsi num_queues. As we're trying not to use underscores in XML, I hope we can still switch it. I haven't found any other num_queues anywhere in the code. Could you point me to the commit that uses that? I'm sending the previously discussed patch in the meantime. Except the virtio-scsi num_queues, there is no other tag for multiple queue yet, we will need a patch to support multiple queue for the network, but it's not committed yet. It's fine to convert it now, 1.0.5 is not released yet. But is it deserved to do, we already have many tags with underscore, which can't be changed for back-compat. I believe those attributes [1] were created by mistake, and kept only because of backward compatibility. I'm trying to be open-minded, though, so I'm not forcing my patch in, but seeing it just as a proposal. Others may have different opinions and I'm willing to discuss that. My first feeling, though, was that we should try to keep the same policy for as many of them as possible. OTOH, I've mistaken the underscore with a hyphen when I remembered what Daniel told me about attributes [2]. I had recalled DV saying something about underscores in the names a long time ago, and I recently asked about underscore vs. camelCase, and danpb said the same thing. (Personally I don't have a preference one way or the other, but if we really are trying to avoid them, now is our chance). I'm fine with either keeping it or changing num_queues. For long term consistence, I agreed with having a consistent naming style is nice. In the meantime, in other device types, we've tried to keep backend details like this pushed into a driver subelement when possible, to avoid polluting the main element (e.g. see the driver subelement of interface). Is it worth putting this numQueues attribute in a driver subelement too? Or am I just playing XML God? Not sure if you mean the upcoming numQueues for interface. But for the existing num_queues, it's for the virtio-scsi controller, putting it in driver doesn't reflect the purpose. But isn't it a backend implementation detail of the specific SCSI controller? In interface and disk, information that is specific to a particular backend (and isn't generally applicable to that type of device) is in the driver subelement. This is the QEMU command line for a virtio-scsi disk: (-device virtio-scsi-pci is mapped to virtio-scsi controller in libvirt XML, with num_queues set): ... -device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \ -usb \ -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-scsi0-0-0-0 \ -device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ /... And this is the QEMU command line for a virtio disk (with event_idx set): ... -drive file=/var/lib/libvirt/images/f14.img,if=none,id=drive-virtio-disk0 \ -device virtio-blk-pci,event_idx=on,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0 \ /... This is the properties the QEMU device scsi-disk supports: % ./x86_64-softmmu/qemu-system-x86_64 -device scsi-disk,? scsi-disk.drive=drive scsi-disk.logical_block_size=blocksize scsi-disk.physical_block_size=blocksize scsi-disk.min_io_size=uint16 scsi-disk.opt_io_size=uint32 scsi-disk.bootindex=int32 scsi-disk.discard_granularity=uint32 scsi-disk.ver=string scsi-disk.serial=string scsi-disk.vendor=string scsi-disk.product=string scsi-disk.removable=on/off scsi-disk.dpofua=on/off scsi-disk.wwn=hex64 scsi-disk.channel=uint32 scsi-disk.scsi-id=uint32 scsi-disk.lun=uint32 And the properties virtio-blk-pci device supports: % ./x86_64-softmmu/qemu-system-x86_64 -device
[libvirt] [PATCH v2 3/5] Add a helper API for setting up a NBD device with qemu-nbd
From: Daniel P. Berrange berra...@redhat.com Add a virFileNBDDeviceAssociate method, which given a filename will setup a NBD device, using qemu-nbd as the server. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/libvirt_private.syms | 1 + src/util/virfile.c | 151 ++- src/util/virfile.h | 6 ++ 3 files changed, 155 insertions(+), 3 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f778e9c..a857a0b 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1272,6 +1272,7 @@ virFileDirectFdFlag; virFileFclose; virFileFdopen; virFileLoopDeviceAssociate; +virFileNBDDeviceAssociate; virFileRewrite; virFileTouch; virFileUpdatePerm; diff --git a/src/util/virfile.c b/src/util/virfile.c index fbaeedd..034648b 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -530,6 +530,7 @@ static int virFileLoopDeviceOpen(char **dev_name) goto cleanup; } +errno = 0; while ((de = readdir(dh)) != NULL) { if (!STRPREFIX(de-d_name, loop)) continue; @@ -561,10 +562,15 @@ static int virFileLoopDeviceOpen(char **dev_name) /* Oh well, try the next device */ VIR_FORCE_CLOSE(fd); VIR_FREE(looppath); +errno = 0; } -virReportError(VIR_ERR_INTERNAL_ERROR, %s, - _(Unable to find a free loop device in /dev)); +if (errno != 0) +virReportSystemError(errno, %s, + _(Unable to iterate over loop devices)); +else +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(Unable to find a free loop device in /dev)); cleanup: if (fd != -1) { @@ -631,10 +637,138 @@ cleanup: return lofd; } + +# define SYSFS_BLOCK_DIR /sys/block + + +static int +virFileNBDDeviceIsBusy(const char *devname) +{ +char *path; +int ret = -1; + +if (virAsprintf(path, SYSFS_BLOCK_DIR /%s/pid, +devname) 0) { +virReportOOMError(); +return -1; +} + +if (access(path, F_OK) 0) { +if (errno == ENOENT) +ret = 0; +else +virReportSystemError(errno, + _(Cannot check NBD device %s pid), + devname); +goto cleanup; +} +ret = 1; + +cleanup: +VIR_FREE(path); +return ret; +} + + +static char * +virFileNBDDeviceFindUnused(void) +{ +DIR *dh; +char *ret = NULL; +struct dirent *de; + +if (!(dh = opendir(SYSFS_BLOCK_DIR))) { +virReportSystemError(errno, + _(Cannot read directory %s), + SYSFS_BLOCK_DIR); +return NULL; +} + +while ((de = readdir(dh)) != NULL) { +if (STRPREFIX(de-d_name, nbd)) { +int rv = virFileNBDDeviceIsBusy(de-d_name); +if (rv 0) +goto cleanup; +if (rv == 0) { +if (virAsprintf(ret, /dev/%s, de-d_name) 0) { +virReportOOMError(); +goto cleanup; +} +goto cleanup; +} +} +} + +virReportSystemError(EBUSY, %s, + _(No free NBD devices)); + +cleanup: +closedir(dh); +return ret; +} + + +int virFileNBDDeviceAssociate(const char *file, + enum virStorageFileFormat fmt, + bool readonly, + char **dev) +{ +char *nbddev; +char *qemunbd; +virCommandPtr cmd = NULL; +int ret = -1; +const char *fmtstr = NULL; + +if (!(nbddev = virFileNBDDeviceFindUnused())) +goto cleanup; + +if (!(qemunbd = virFindFileInPath(qemu-nbd))) { +virReportSystemError(ENOENT, %s, + _(Unable to find 'qemu-nbd' binary in $PATH)); +goto cleanup; +} + +if (fmt 0) +fmtstr = virStorageFileFormatTypeToString(fmt); + +cmd = virCommandNew(qemunbd); + +/* Explicitly not trying to cope with old qemu-nbd which + * lacked --format. We want to see a fatal error in that + * case since it would be security flaw to continue */ +if (fmtstr) { +virCommandAddArg(cmd, --format); +virCommandAddArg(cmd, fmtstr); +} + +if (readonly) +virCommandAddArg(cmd, -r); + +virCommandAddArgList(cmd, + -n, /* Don't cache in qemu-nbd layer */ + -c, nbddev, + file, NULL); + +/* qemu-nbd will daemonize itself */ + +if (virCommandRun(cmd, NULL) 0) +goto cleanup; + +*dev = nbddev; +nbddev = NULL; +ret = 0; + +cleanup: +VIR_FREE(nbddev); +VIR_FREE(qemunbd); +virCommandFree(cmd); +return ret; +} + #else /* __linux__ */ int virFileLoopDeviceAssociate(const char *file, - char
Re: [libvirt] [PATCH] Use camelCase for XML attribute numQueues
On 04/22/2013 06:58 AM, Daniel Veillard wrote: On Thu, Apr 18, 2013 at 08:27:56AM -0600, Eric Blake wrote: On 04/18/2013 03:02 AM, Martin Kletzander wrote: In commit d4bf0a9, we used num_queues for an attribute in the XML, but the consensus is that we use camelCase for that. Since there was no release yet (the above commit describes as v1.0.4-65-gd4bf0a9), we still have time to change it. You may want to wait for DV's opinion on naming, but I wasn't aware that we had a consensus on camelCase. In fact, '_' is currently winning, although it's hard to say whether that is limited to older interfaces. Hum, I don't think I ever suggested such rules at the XML level. We have that convention for code, as eric pointed out it is not as clear for XML (and unfortunately we can't hamonize at this point without quite a bit of churn - we would have to support the old syntax forever anyway). Yeah, I don't think there's anything to gain (and lots to lose!) from changing attributes that have already been in a release. And I think I first heard about dislike of underscore in XML from danpb (and incorrectly extrapolated that to you: https://www.redhat.com/archives/libvir-list/2013-April/msg01359.html). Of course in this case, we *could* avoid the camelCase vs underscore entirely by just naming the attribute queues instead of numQueues or num_queues (the fact that the attribute is a number makes it pretty obvious that it is the number of queues). Since I first noticed the name, I'm actually more concerned that the attribute is in the toplevel of controller rather than in a driver subelement. It's true that this would be the first driver item under controller, but more backend-specific tuning attributes may follow, and it would be good to have them all collected together in the same place. Since I've already brought this up in another thread (same message as referenced above), I'll drop this if there's no response, but does anyone else have an opinion about that? I don't have a strong opinion on _ vs. camelcase for markup, I guess the exact same arguments can be raised pros/cons on visibility and existing use :-\ Seems Eric is rather adverse to _ which was looking like the winner, I dislike '-' myself as it is used for comments, and very uncommon in identifiers in XML (well it's barred from being in an identifier in most language). If Eric really can't stand '_' then maybe the simplest is really to start standardizing on camelCase, I don't have a preference either, but think that we should (as much as possible) try to stick with one or the other, at least for new attributes. A decision followed by a written rule in HACKING would be a good way to make that happen. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rpc: message related sizes enlarged
From: Daniel Hansel daniel.han...@linux.vnet.ibm.com We have seen an issue on s390x platform where domain XMLs larger than 1MB were used. The define command was finished successfully. The dumpxml command was not successful (i.e. could not encode message payload). Enlarged message related sizes (e.g. maximum string size, message size, etc.) to handle larger system configurations used on s390x platform. Signed-off-by: Daniel Hansel daniel.han...@linux.vnet.ibm.com Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com --- src/remote/remote_protocol.x |6 +++--- src/rpc/virnetprotocol.x |6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index b957b8e..d492bf3 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -65,7 +65,7 @@ * This is an arbitrary limit designed to stop the decoder from trying * to allocate unbounded amounts of memory when fed with a bad message. */ -const REMOTE_STRING_MAX = 1048576; +const REMOTE_STRING_MAX = 4194304; /* A long string, which may NOT be NULL. */ typedef string remote_nonnull_stringREMOTE_STRING_MAX; @@ -160,13 +160,13 @@ const REMOTE_DOMAIN_SNAPSHOT_LIST_NAMES_MAX = 1024; * Note applications need to be aware of this limit and issue multiple * requests for large amounts of data. */ -const REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX = 1048576; +const REMOTE_DOMAIN_BLOCK_PEEK_BUFFER_MAX = 4194304; /* Maximum length of a memory peek buffer message. * Note applications need to be aware of this limit and issue multiple * requests for large amounts of data. */ -const REMOTE_DOMAIN_MEMORY_PEEK_BUFFER_MAX = 1048576; +const REMOTE_DOMAIN_MEMORY_PEEK_BUFFER_MAX = 4194304; /* * Maximum length of a security label list. diff --git a/src/rpc/virnetprotocol.x b/src/rpc/virnetprotocol.x index eb2e81d..7d5557c 100644 --- a/src/rpc/virnetprotocol.x +++ b/src/rpc/virnetprotocol.x @@ -45,13 +45,13 @@ /*- Data types. -*/ /* Maximum total message size (serialised). */ -const VIR_NET_MESSAGE_MAX = 4194304; +const VIR_NET_MESSAGE_MAX = 16777216; /* Size of struct virNetMessageHeader (serialised)*/ const VIR_NET_MESSAGE_HEADER_MAX = 24; /* Size of message payload */ -const VIR_NET_MESSAGE_PAYLOAD_MAX = 4194280; +const VIR_NET_MESSAGE_PAYLOAD_MAX = 16777192; /* Size of message length field. Not counted in VIR_NET_MESSAGE_MAX */ const VIR_NET_MESSAGE_LEN_MAX = 4; @@ -60,7 +60,7 @@ const VIR_NET_MESSAGE_LEN_MAX = 4; * This is an arbitrary limit designed to stop the decoder from trying * to allocate unbounded amounts of memory when fed with a bad message. */ -const VIR_NET_MESSAGE_STRING_MAX = 1048576; +const VIR_NET_MESSAGE_STRING_MAX = 4194304; /* Limit on number of File Descriptors allowed to be * passed per message -- 1.7.9.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH-v4 1/2] create virSocketAddrGetIpPrefix utility function
On 04/20/2013 03:45 PM, Gene Czarcinski wrote: Create the utility function virSocketAddrGetIpPrefix() to determine the prefix for this network. The code in this function was adapted from virNetworkIpDefPrefix(). Update virNetworkIpDefPrefix() in src/conf/network_conf.c to use the new utility function. . Signed-off-by: Gene Czarcinski g...@czarc.net --- ACK. I'll push it after I've reviewed 2/2. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2 4/5] Add 'nbd' as a valid filesystem driver type
From: Daniel P. Berrange berra...@redhat.com The filesystem element can now accept a driver type='nbd'/ as an alternative to 'loop'. The benefit of NBD is support for non-raw disk image formats. Signed-off-by: Daniel P. Berrange berra...@redhat.com --- docs/formatdomain.html.in | 5 +++-- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c| 3 ++- src/conf/domain_conf.h| 1 + src/qemu/qemu_command.c | 1 + 5 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a707cc8..2eca6d8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -1985,8 +1985,9 @@ the codetype/code attribute selects the primary backend driver name, while the codeformat/code attribute provides the format type. For example, LXC -supports a type of loop, with a format of raw. QEMU -supports a type of path or handle, but no formats. +supports a type of loop, with a format of raw or +nbd with any format. QEMU supports a type of path +or handle, but no formats. /li /ul /dd diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 206757f..33fac33 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1659,6 +1659,7 @@ valuepath/value valuehandle/value valueloop/value +valuenbd/value /choice /attribute /optional diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 582308b..51ad42c 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -337,7 +337,8 @@ VIR_ENUM_IMPL(virDomainFSDriverType, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, default, path, handle, - loop) + loop, + nbd) VIR_ENUM_IMPL(virDomainFSAccessMode, VIR_DOMAIN_FS_ACCESSMODE_LAST, passthrough, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index a65aabc..3338ddf 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -768,6 +768,7 @@ enum virDomainFSDriverType { VIR_DOMAIN_FS_DRIVER_TYPE_PATH, VIR_DOMAIN_FS_DRIVER_TYPE_HANDLE, VIR_DOMAIN_FS_DRIVER_TYPE_LOOP, +VIR_DOMAIN_FS_DRIVER_TYPE_NBD, VIR_DOMAIN_FS_DRIVER_TYPE_LAST }; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 05c2a41..148e24e 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -131,6 +131,7 @@ VIR_ENUM_IMPL(qemuDomainFSDriver, VIR_DOMAIN_FS_DRIVER_TYPE_LAST, local, local, handle, + NULL, NULL); -- 1.7.11.7 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 08/12] qemu: Rework qemuNetworkIfaceConnect
For future work it's better, if tapfd is passed as pointer. Moreover, we need to be able to return multiple values now. --- src/qemu/qemu_command.c | 70 + src/qemu/qemu_command.h | 4 ++- src/qemu/qemu_hotplug.c | 4 +-- 3 files changed, 41 insertions(+), 37 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 7337069..58c81d7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -199,11 +199,12 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, virConnectPtr conn, virQEMUDriverPtr driver, virDomainNetDefPtr net, -virQEMUCapsPtr qemuCaps) +virQEMUCapsPtr qemuCaps, +int *tapfd, +int tapfdSize) { char *brname = NULL; -int err; -int tapfd = -1; +int ret = -1; unsigned int tap_create_flags = VIR_NETDEV_TAP_CREATE_IFUP; bool template_ifname = false; int actualType = virDomainNetGetActualType(net); @@ -215,7 +216,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, virNetworkPtr network = virNetworkLookupByName(conn, net-data.network.name); if (!network) -return -1; +return ret; active = virNetworkIsActive(network); if (active != 1) { @@ -240,18 +241,18 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, virFreeError(errobj); if (fail) -return -1; +return ret; } else if (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { if (!(brname = strdup(virDomainNetGetActualBridgeName(net { virReportOOMError(); -return -1; +return ret; } } else { virReportError(VIR_ERR_INTERNAL_ERROR, _(Network type %d is not supported), virDomainNetGetActualType(net)); -return -1; +return ret; } if (!net-ifname || @@ -271,49 +272,51 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, tap_create_flags |= VIR_NETDEV_TAP_CREATE_VNET_HDR; } -err = virNetDevTapCreateInBridgePort(brname, net-ifname, net-mac, - def-uuid, tapfd, 1, - virDomainNetGetActualVirtPortProfile(net), - virDomainNetGetActualVlan(net), - tap_create_flags); -virDomainAuditNetDevice(def, net, /dev/net/tun, tapfd = 0); -if (err 0) { +if (virNetDevTapCreateInBridgePort(brname, net-ifname, net-mac, + def-uuid, tapfd, tapfdSize, + virDomainNetGetActualVirtPortProfile(net), + virDomainNetGetActualVlan(net), + tap_create_flags) 0) { +virDomainAuditNetDevice(def, net, /dev/net/tun, false); if (template_ifname) VIR_FREE(net-ifname); -tapfd = -1; +goto cleanup; } +virDomainAuditNetDevice(def, net, /dev/net/tun, true); -if (cfg-macFilter) { -if ((err = networkAllowMacOnPort(driver, net-ifname, net-mac))) { -virReportSystemError(err, - _(failed to add ebtables rule to allow MAC address on '%s'), - net-ifname); -} +if (cfg-macFilter +(ret = networkAllowMacOnPort(driver, net-ifname, net-mac)) 0) { +virReportSystemError(ret, + _(failed to add ebtables rule + to allow MAC address on '%s'), + net-ifname); } -if (tapfd = 0 -virNetDevBandwidthSet(net-ifname, +if (virNetDevBandwidthSet(net-ifname, virDomainNetGetActualBandwidth(net), false) 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _(cannot set bandwidth limits on %s), net-ifname); -VIR_FORCE_CLOSE(tapfd); goto cleanup; } -if (tapfd = 0) { -if ((net-filter) (net-ifname)) { -if (virDomainConfNWFilterInstantiate(conn, def-uuid, net) 0) -VIR_FORCE_CLOSE(tapfd); -} -} +if (net-filter net-ifname +virDomainConfNWFilterInstantiate(conn, def-uuid, net) 0) +goto cleanup; + + +ret = 0; cleanup: +if (ret 0) { +while (tapfdSize--) +VIR_FORCE_CLOSE(tapfd[tapfdSize]); +} VIR_FREE(brname); virObjectUnref(cfg); -return tapfd; +return ret; } @@ -5868,9 +5871,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
[libvirt] [PATCH v1 04/12] qemu: Make qemuMonitorAddHostNetwork to pass multiple FDs
Currently, only one tapfd and one vhostfd could be passed. However, multiqueue network requires several FDs to be passed to qemu so we must adapt out monitor handling functions to cope with that. --- src/qemu/qemu_hotplug.c | 7 +-- src/qemu/qemu_monitor.c | 39 +++ src/qemu/qemu_monitor.h | 4 ++-- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 254ab4a..d1347c6 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -830,8 +830,11 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, goto cleanup; } } else { -if (qemuMonitorAddHostNetwork(priv-mon, netstr, tapfd, tapfd_name, - vhostfd, vhostfd_name) 0) { +if (qemuMonitorAddHostNetwork(priv-mon, netstr, + tapfd, tapfd_name, + tapfd_name ? 1 : 0, + vhostfd, vhostfd_name, + vhostfd_name ? 1 : 0) 0) { qemuDomainObjExitMonitor(driver, vm); virDomainAuditNet(vm, NULL, net, attach, false); goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 4ca80f4..763dcb2 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2460,14 +2460,16 @@ cleanup: int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, const char *netstr, - int tapfd, const char *tapfd_name, - int vhostfd, const char *vhostfd_name) + int *tapfd, char **tapfdName, int tapfdSize, + int *vhostfd, char **vhostfdName, int vhostfdSize) { int ret = -1; -VIR_DEBUG(mon=%p netstr=%s tapfd=%d tapfd_name=%s - vhostfd=%d vhostfd_name=%s, - mon, netstr, tapfd, NULLSTR(tapfd_name), - vhostfd, NULLSTR(vhostfd_name)); +int i = 0, j = 0; + +VIR_DEBUG(mon=%p netstr=%s tapfd=%p tapfdName=%p tapfdSize=%d + vhostfd=%p vhostfdName=%p vhostfdSize=%d, + mon, netstr, tapfd, tapfdName, tapfdSize, + vhostfd, vhostfdName, vhostfdSize); if (!mon) { virReportError(VIR_ERR_INVALID_ARG, %s, @@ -2475,12 +2477,13 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, return -1; } -if (tapfd = 0 qemuMonitorSendFileHandle(mon, tapfd_name, tapfd) 0) -return -1; -if (vhostfd = 0 -qemuMonitorSendFileHandle(mon, vhostfd_name, vhostfd) 0) { -vhostfd = -1; -goto cleanup; +for (i = 0; i tapfdSize; i++) { +if (qemuMonitorSendFileHandle(mon, tapfdName[i], tapfd[i]) 0) +goto cleanup; +} +for (j = 0; j vhostfdSize; j++) { +if (qemuMonitorSendFileHandle(mon, vhostfdName[j], vhostfd[j]) 0) +goto cleanup; } if (mon-json) @@ -2491,10 +2494,14 @@ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, cleanup: if (ret 0) { -if (tapfd = 0 qemuMonitorCloseFileHandle(mon, tapfd_name) 0) -VIR_WARN(failed to close device handle '%s', tapfd_name); -if (vhostfd = 0 qemuMonitorCloseFileHandle(mon, vhostfd_name) 0) -VIR_WARN(failed to close device handle '%s', vhostfd_name); +while (i--) { +if (qemuMonitorCloseFileHandle(mon, tapfdName[i]) 0) +VIR_WARN(failed to close device handle '%s', tapfdName[i]); +} +while (j--) { +if (qemuMonitorCloseFileHandle(mon, vhostfdName[j]) 0) +VIR_WARN(failed to close device handle '%s', vhostfdName[j]); +} } return ret; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index 580e42b..45257a4 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -497,8 +497,8 @@ int qemuMonitorRemoveFd(qemuMonitorPtr mon, int fdset, int fd); */ int qemuMonitorAddHostNetwork(qemuMonitorPtr mon, const char *netstr, - int tapfd, const char *tapfd_name, - int vhostfd, const char *vhostfd_name); + int *tapfd, char **tapfdName, int tapfdSize, + int *vhostfd, char **vhostfdName, int vhostfdSize); int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, int vlan, -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 03/12] qemu: Make qemuMonitorAddHostNetwork to pass multiple FDs
Currently, only one tapfd and one vhostfd could be passed. However, multiqueue network requires several FDs to be passed to qemu so we must adapt out monitor handling functions to cope with that. --- src/qemu/qemu_hotplug.c | 7 +-- src/qemu/qemu_monitor.c | 39 +++ src/qemu/qemu_monitor.h | 4 ++-- 3 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 238d0d7..254ab4a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -820,8 +820,11 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, qemuDomainObjEnterMonitor(driver, vm); if (virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NETDEV) virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) { -if (qemuMonitorAddNetdev(priv-mon, netstr, tapfd, tapfd_name, - vhostfd, vhostfd_name) 0) { +if (qemuMonitorAddNetdev(priv-mon, netstr, + tapfd, tapfd_name, + tapfd_name ? 1 : 0, + vhostfd, vhostfd_name, + vhostfd_name ? 1 : 0) 0) { qemuDomainObjExitMonitor(driver, vm); virDomainAuditNet(vm, NULL, net, attach, false); goto cleanup; diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c index 1b1d4a1..4ca80f4 100644 --- a/src/qemu/qemu_monitor.c +++ b/src/qemu/qemu_monitor.c @@ -2526,14 +2526,16 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, int qemuMonitorAddNetdev(qemuMonitorPtr mon, const char *netdevstr, - int tapfd, const char *tapfd_name, - int vhostfd, const char *vhostfd_name) + int *tapfd, char **tapfdName, int tapfdSize, + int *vhostfd, char **vhostfdName, int vhostfdSize) { int ret = -1; -VIR_DEBUG(mon=%p netdevstr=%s tapfd=%d tapfd_name=%s - vhostfd=%d vhostfd_name=%s, - mon, netdevstr, tapfd, NULLSTR(tapfd_name), - vhostfd, NULLSTR(vhostfd_name)); +int i = 0, j = 0; + +VIR_DEBUG(mon=%p netdevstr=%s tapfd=%p tapfdName=%p tapfdSize=%d + vhostfd=%p vhostfdName=%p vhostfdSize=%d, + mon, netdevstr, tapfd, tapfdName, tapfdSize, + vhostfd, vhostfdName, tapfdSize); if (!mon) { virReportError(VIR_ERR_INVALID_ARG, %s, @@ -2541,12 +2543,13 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon, return -1; } -if (tapfd = 0 qemuMonitorSendFileHandle(mon, tapfd_name, tapfd) 0) -return -1; -if (vhostfd = 0 -qemuMonitorSendFileHandle(mon, vhostfd_name, vhostfd) 0) { -vhostfd = -1; -goto cleanup; +for (i = 0; i tapfdSize; i++) { +if (qemuMonitorSendFileHandle(mon, tapfdName[i], tapfd[i]) 0) +goto cleanup; +} +for (j = 0; j vhostfdSize; j++) { +if (qemuMonitorSendFileHandle(mon, vhostfdName[j], vhostfd[j]) 0) +goto cleanup; } if (mon-json) @@ -2556,10 +2559,14 @@ int qemuMonitorAddNetdev(qemuMonitorPtr mon, cleanup: if (ret 0) { -if (tapfd = 0 qemuMonitorCloseFileHandle(mon, tapfd_name) 0) -VIR_WARN(failed to close device handle '%s', tapfd_name); -if (vhostfd = 0 qemuMonitorCloseFileHandle(mon, vhostfd_name) 0) -VIR_WARN(failed to close device handle '%s', vhostfd_name); +while (i--) { +if (qemuMonitorCloseFileHandle(mon, tapfdName[i]) 0) +VIR_WARN(failed to close device handle '%s', tapfdName[i]); +} +while (j--) { +if (qemuMonitorCloseFileHandle(mon, vhostfdName[j]) 0) +VIR_WARN(failed to close device handle '%s', vhostfdName[j]); +} } return ret; diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h index f39f009..580e42b 100644 --- a/src/qemu/qemu_monitor.h +++ b/src/qemu/qemu_monitor.h @@ -506,8 +506,8 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon, int qemuMonitorAddNetdev(qemuMonitorPtr mon, const char *netdevstr, - int tapfd, const char *tapfd_name, - int vhostfd, const char *vhostfd_name); + int *tapfd, char **tapfdName, int tapfdSize, + int *vhostfd, char **vhostfdName, int vhostfdSize); int qemuMonitorRemoveNetdev(qemuMonitorPtr mon, const char *alias); -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 01/12] Introduce /domain/devices/interface/driver/@queues attribute
This attribute is going to represent number of queues for multique vhost network interface. This commit implements XML extension part of the feature and add one test as well. For now, we can only do xml2xml test as qemu command line generation code is not adapted yet. --- docs/formatdomain.html.in | 11 - docs/schemas/domaincommon.rng | 5 +++ src/conf/domain_conf.c | 15 +++ src/conf/domain_conf.h | 1 + src/qemu/qemu_domain.c | 27 +++- tests/qemuxml2argvdata/qemuxml2argv-event_idx.xml | 2 +- .../qemuxml2argv-net-virtio-device.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-vhost_queues.xml | 50 ++ tests/qemuxml2argvdata/qemuxml2argv-virtio-lun.xml | 2 +- tests/qemuxml2xmltest.c| 1 + 10 files changed, 102 insertions(+), 14 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-vhost_queues.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 888c005..34454e5 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3070,7 +3070,7 @@ qemu-kvm -net nic,model=? /dev/null lt;source network='default'/gt; lt;target dev='vnet1'/gt; lt;model type='virtio'/gt; - blt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off'/gt;/b + blt;driver name='vhost' txmode='iothread' ioeventfd='on' event_idx='off' queues='5'/gt;/b lt;/interfacegt; lt;/devicesgt; .../pre @@ -3151,6 +3151,15 @@ qemu-kvm -net nic,model=? /dev/null bIn general you should leave this option alone, unless you are very certain you know what you are doing./b /dd + dtcodequeues/code/dt + dd +The optional codequeues/code attribute controls number of +queues for a href=http://www.linux-kvm.org/page/Multiqueue;M +ultiqueue virtio-net/a feature. Long story short, in case of a +virtio net device, multiple queues can be created so each queue is +handled by different processor resulting in much higher throughput. +span class=sinceSince 1.0.5 (QEMU and KVM only)/span + /dd /dl h5a name=elementsNICSTargetOverrideOverriding the target element/a/h5 diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..8d02e6d 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1932,6 +1932,11 @@ /attribute /optional optional +attribute name='queues' + ref name=positiveInteger/ +/attribute + /optional + optional attribute name=txmode choice valueiothread/value diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dc0ecaa..4b2f84a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -5691,6 +5691,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, char *txmode = NULL; char *ioeventfd = NULL; char *event_idx = NULL; +char *queues = NULL; char *filter = NULL; char *internal = NULL; char *devaddr = NULL; @@ -5802,6 +5803,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, txmode = virXMLPropString(cur, txmode); ioeventfd = virXMLPropString(cur, ioeventfd); event_idx = virXMLPropString(cur, event_idx); +queues = virXMLPropString(cur, queues); } else if (xmlStrEqual(cur-name, BAD_CAST filterref)) { if (filter) { virReportError(VIR_ERR_XML_ERROR, %s, @@ -6092,6 +6094,16 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } def-driver.virtio.event_idx = idx; } +if (queues) { +unsigned int q; +if (virStrToLong_ui(queues, NULL, 10, q) 0) { +virReportError(VIR_ERR_XML_DETAIL, + _('queues' attribute must be unsigned integer: %s), + queues); +goto error; +} +def-driver.virtio.queues = q; +} } def-linkstate = VIR_DOMAIN_NET_INTERFACE_LINK_STATE_DEFAULT; @@ -6145,6 +6157,7 @@ cleanup: VIR_FREE(txmode); VIR_FREE(ioeventfd); VIR_FREE(event_idx); +VIR_FREE(queues); VIR_FREE(filter); VIR_FREE(type); VIR_FREE(internal); @@ -13967,6 +13980,8 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferAsprintf(buf, event_idx='%s', virDomainVirtioEventIdxTypeToString(def-driver.virtio.event_idx)); } +if (def-driver.virtio.queues) +virBufferAsprintf(buf, queues='%u', def-driver.virtio.queues); virBufferAddLit(buf, /\n); } } diff --git
[libvirt] [PATCH v1 12/12] qemu: Enable multiqueue network
--- src/qemu/qemu_command.c | 10 ++ src/qemu/qemu_hotplug.c | 5 +++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 60873c7..a7d422a 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5887,12 +5887,13 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || cfg-privileged || (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) { -if (VIR_ALLOC(tapfd) 0 || VIR_ALLOC(tapfdName) 0) { +if (VIR_ALLOC_N(tapfd, net-driver.virtio.queues) 0 || +VIR_ALLOC_N(tapfdName, net-driver.virtio.queues) 0) { virReportOOMError(); goto cleanup; } -tapfdSize = 1; +tapfdSize = net-driver.virtio.queues; if (qemuNetworkIfaceConnect(def, conn, driver, net, qemuCaps, tapfd, tapfdSize) 0) goto cleanup; @@ -5916,11 +5917,12 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, int useVhost; /* Attempt to use vhost-net mode for these types of network device */ -if (VIR_ALLOC(vhostfd) 0 || VIR_ALLOC(vhostfdName)) { +if (VIR_ALLOC_N(vhostfd, net-driver.virtio.queues) 0 || +VIR_ALLOC_N(vhostfdName, net-driver.virtio.queues)) { virReportOOMError(); goto cleanup; } -vhostfdSize = 1; +vhostfdSize = 1 + net-driver.virtio.queues; useVhost = qemuOpenVhostNet(def, net, qemuCaps, vhostfd, vhostfdSize); if (useVhost 0) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3a00811..d35801d 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -743,11 +743,12 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || cfg-privileged || (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) { -if (VIR_ALLOC(tapfd) 0 || VIR_ALLOC(vhostfd) 0) { +if (VIR_ALLOC_N(tapfd, net-driver.virtio.queues) 0 || +VIR_ALLOC_N(vhostfd, net-driver.virtio.queues) 0) { virReportOOMError(); goto cleanup; } -tapfdSize = vhostfdSize = 1; +tapfdSize = vhostfdSize = net-driver.virtio.queues; if (qemuNetworkIfaceConnect(vm-def, conn, driver, net, priv-qemuCaps, tapfd, tapfdSize) 0) goto cleanup; -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 06/12] util: Learn virNetDevTapCreate multiqueue network
Currently, the function knows how to open a TAP device for a single time. However, in case of multiqueue network we need to open it multiple times. Moreover, when doing TUNSETIFF ioctl, the IFF_MULTI_QUEUE flag shall be requested. This commit changes a behaviour slightly as well. Till now it was possible to pass NULL as an @fd address by which caller didn't care about returned FD. While this was used only in UML driver, the appropriate UML code snippets has been changed as well. --- src/network/bridge_driver.c | 2 +- src/qemu/qemu_command.c | 2 +- src/uml/uml_conf.c | 5 ++- src/util/virnetdevtap.c | 105 src/util/virnetdevtap.h | 2 + 5 files changed, 66 insertions(+), 50 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 27dd230..7118702 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2424,7 +2424,7 @@ networkStartNetworkVirtual(struct network_driver *driver, /* Keep tun fd open and interface up to allow for IPv6 DAD to happen */ if (virNetDevTapCreateInBridgePort(network-def-bridge, macTapIfName, network-def-mac, - NULL, tapfd, NULL, NULL, + NULL, tapfd, 1, NULL, NULL, VIR_NETDEV_TAP_CREATE_USE_MAC_FOR_BRIDGE | VIR_NETDEV_TAP_CREATE_IFUP | VIR_NETDEV_TAP_CREATE_PERSIST) 0) { diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b5374e0..f5d11ad 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -272,7 +272,7 @@ qemuNetworkIfaceConnect(virDomainDefPtr def, } err = virNetDevTapCreateInBridgePort(brname, net-ifname, net-mac, - def-uuid, tapfd, + def-uuid, tapfd, 1, virDomainNetGetActualVirtPortProfile(net), virDomainNetGetActualVlan(net), tap_create_flags); diff --git a/src/uml/uml_conf.c b/src/uml/uml_conf.c index 4fa7927..4816f06 100644 --- a/src/uml/uml_conf.c +++ b/src/uml/uml_conf.c @@ -110,6 +110,7 @@ umlConnectTapDevice(virConnectPtr conn, const char *bridge) { bool template_ifname = false; +int tapfd; if (!net-ifname || STRPREFIX(net-ifname, VIR_NET_GENERATED_PREFIX) || @@ -122,7 +123,7 @@ umlConnectTapDevice(virConnectPtr conn, } if (virNetDevTapCreateInBridgePort(bridge, net-ifname, net-mac, - vm-uuid, NULL, + vm-uuid, tapfd, 1, virDomainNetGetActualVirtPortProfile(net), virDomainNetGetActualVlan(net), VIR_NETDEV_TAP_CREATE_IFUP | @@ -140,11 +141,13 @@ umlConnectTapDevice(virConnectPtr conn, } } +VIR_FORCE_CLOSE(tapfd); return 0; no_memory: virReportOOMError(); error: +VIR_FORCE_CLOSE(tapfd); return -1; } diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index e4ce223..cdf6bd3 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -106,7 +106,8 @@ virNetDevProbeVnetHdr(int tapfd) /** * virNetDevTapCreate: * @ifname: the interface name - * @tapfd: file descriptor return value for the new tap device + * @tapfds: file descriptor return value for the new tap device + * @tapfdSize: size of @tapfd * @flags: OR of virNetDevTapCreateFlags. Only one flag is recognized: * * VIR_NETDEV_TAP_CREATE_VNET_HDR @@ -114,76 +115,83 @@ virNetDevProbeVnetHdr(int tapfd) * VIR_NETDEV_TAP_CREATE_PERSIST * - The device will persist after the file descriptor is closed * - * Creates a tap interface. - * If the @tapfd parameter is supplied, the open tap device file descriptor - * will be returned, otherwise the TAP device will be closed. The caller must - * use virNetDevTapDelete to remove a persistent TAP device when it is no - * longer needed. + * Creates a tap interface. The caller must use virNetDevTapDelete to + * remove a persistent TAP device when it is no longer needed. In case + * @tapfdSize is greater than one, multiqueue extension is requested + * from kernel. * * Returns 0 in case of success or -1 on failure. */ int virNetDevTapCreate(char **ifname, int *tapfd, + int tapfdSize, unsigned int flags) { -int fd; +int i; struct ifreq ifr; int ret = -1; -if ((fd = open(/dev/net/tun, O_RDWR)) 0) { -virReportSystemError(errno, %s, - _(Unable to open /dev/net/tun, is
[libvirt] [PATCH v1 02/12] qemu: Move interface cmd line construction into a separate function
Currently, we have one huge function to construct qemu command line. This is very ineffective esp. if there's a fault somewhere. --- src/qemu/qemu_command.c | 323 +--- 1 file changed, 171 insertions(+), 152 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 05c12b2..4af2d04 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5751,6 +5751,172 @@ error: return -1; } +static int +qemuBuildInterfaceCommandLine(virCommandPtr cmd, + virQEMUDriverPtr driver, + virConnectPtr conn, + virDomainDefPtr def, + virDomainNetDefPtr net, + virQEMUCapsPtr qemuCaps, + int vlan, + int bootNet, + enum virNetDevVPortProfileOp vmop) +{ +int ret = -1; +int tapfd = -1; +char *nic = NULL, *host = NULL; +char *tapfdName = NULL; +char *vhostfdName = NULL; +int bootindex = bootNet; +int actualType; +virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + +if (!bootindex) +bootindex = net-info.bootIndex; + +/* If appropriate, grab a physical device from the configured + * network's pool of devices, or resolve bridge device name + * to the one defined in the network definition. + */ +if (networkAllocateActualDevice(net) 0) +goto cleanup; + +actualType = virDomainNetGetActualType(net); +if (actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV) { +if (net-type == VIR_DOMAIN_NET_TYPE_NETWORK) { +virDomainHostdevDefPtr hostdev = virDomainNetGetActualHostdev(net); +virDomainHostdevDefPtr found; +/* For a network with forward mode='hostdev', there is a need to + * add the newly minted hostdev to the hostdevs array. + */ +if (qemuAssignDeviceHostdevAlias(def, hostdev, def-nhostdevs-1) 0) +goto cleanup; + +if (virDomainHostdevFind(def, hostdev, found) 0) { +if (virDomainHostdevInsert(def, hostdev) 0) { +virReportOOMError(); +goto cleanup; +} +if (qemuPrepareHostdevPCIDevices(driver, def-name, def-uuid, + hostdev, 1) 0) { +goto cleanup; +} +} else { +virReportError(VIR_ERR_INTERNAL_ERROR, + _(PCI device %04x:%02x:%02x.%x + allocated from network %s is already + in use by domain %s), + hostdev-source.subsys.u.pci.domain, + hostdev-source.subsys.u.pci.bus, + hostdev-source.subsys.u.pci.slot, + hostdev-source.subsys.u.pci.function, + net-data.network.name, + def-name); +goto cleanup; +} +} +/* hostdev interface doesn't require nor -net nor -netdev */ +ret = 0; +goto cleanup; +} + +if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || +actualType == VIR_DOMAIN_NET_TYPE_BRIDGE) { +/* + * If type='bridge' then we attempt to allocate the tap fd here only if + * running under a privilged user or -netdev bridge option is not + * supported. + */ +if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || +cfg-privileged || +(!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) { +tapfd = qemuNetworkIfaceConnect(def, conn, driver, net, +qemuCaps); +if (tapfd 0) +goto cleanup; +} +} else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { +tapfd = qemuPhysIfaceConnect(def, driver, net, + qemuCaps, vmop); +if (tapfd 0) +goto cleanup; +} + +if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || +actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || +actualType == VIR_DOMAIN_NET_TYPE_ETHERNET || +actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { +/* Attempt to use vhost-net mode for these types of + network device */ +int vhostfd; + +if (qemuOpenVhostNet(def, net, qemuCaps, vhostfd) 0) +goto cleanup; +if (vhostfd = 0) { +virCommandTransferFD(cmd, vhostfd); + +if (virAsprintf(vhostfdName, %d, vhostfd) 0) { +virReportOOMError(); +goto cleanup; +} +} +} + +if (tapfd = 0) { +virCommandTransferFD(cmd, tapfd); + +if
[libvirt] [PATCH v1 09/12] qemu: Adapt qemuDomainAttachNetDevice to multiqueue net
--- src/qemu/qemu_hotplug.c | 96 - 1 file changed, 63 insertions(+), 33 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 3505c52..3a00811 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -683,10 +683,12 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, virDomainNetDefPtr net) { qemuDomainObjPrivatePtr priv = vm-privateData; -char *tapfd_name = NULL; -int tapfd = -1; -char *vhostfd_name = NULL; -int vhostfd = -1; +char **tapfdName = NULL; +int *tapfd = NULL; +int tapfdSize = 0; +char **vhostfdName = NULL; +int *vhostfd = NULL; +int vhostfdSize = 0; char *nicstr = NULL; char *netstr = NULL; virNetDevVPortProfilePtr vport = NULL; @@ -697,6 +699,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, bool iface_connected = false; int actualType; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +int i; /* preallocate new slot for device */ if (VIR_REALLOC_N(vm-def-nets, vm-def-nnets+1) 0) { @@ -740,23 +743,38 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || cfg-privileged || (!virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) { +if (VIR_ALLOC(tapfd) 0 || VIR_ALLOC(vhostfd) 0) { +virReportOOMError(); +goto cleanup; +} +tapfdSize = vhostfdSize = 1; if (qemuNetworkIfaceConnect(vm-def, conn, driver, net, -priv-qemuCaps, tapfd, 1) 0) +priv-qemuCaps, tapfd, tapfdSize) 0) goto cleanup; iface_connected = true; -if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd, 1) 0) +if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd, vhostfdSize) 0) goto cleanup; } } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { -if ((tapfd = qemuPhysIfaceConnect(vm-def, driver, net, - priv-qemuCaps, - VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) 0) +if (VIR_ALLOC(tapfd) 0 || VIR_ALLOC(vhostfd) 0) { +virReportOOMError(); +goto cleanup; +} +tapfdSize = vhostfdSize = 1; +if ((tapfd[0] = qemuPhysIfaceConnect(vm-def, driver, net, + priv-qemuCaps, + VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) 0) goto cleanup; iface_connected = true; -if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd, 1) 0) +if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd, vhostfdSize) 0) goto cleanup; } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { -if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd, 1) 0) +if (VIR_ALLOC(vhostfd) 0) { +virReportOOMError(); +goto cleanup; +} +vhostfdSize = 1; +if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd, vhostfdSize) 0) goto cleanup; } @@ -794,13 +812,19 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, } } -if (tapfd != -1) { -if (virAsprintf(tapfd_name, fd-%s, net-info.alias) 0) +if (VIR_ALLOC_N(tapfdName, tapfdSize) 0 || +VIR_ALLOC_N(vhostfdName, vhostfdSize) 0) { +virReportOOMError(); +goto cleanup; +} + +for (i = 0; i tapfdSize; i++) { +if (virAsprintf(tapfdName[i], fd-%s%d, net-info.alias, i) 0) goto no_memory; } -if (vhostfd != -1) { -if (virAsprintf(vhostfd_name, vhostfd-%s, net-info.alias) 0) +for (i = 0; i vhostfdSize; i++) { +if (virAsprintf(vhostfdName[i], vhostfd-%s%d, net-info.alias, i) 0) goto no_memory; } @@ -808,14 +832,14 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, virQEMUCapsGet(priv-qemuCaps, QEMU_CAPS_DEVICE)) { if (!(netstr = qemuBuildHostNetStr(net, driver, priv-qemuCaps, ',', -1, - tapfd_name, tapfd_name ? 1 : 0, - vhostfd_name, vhostfd_name ? 1 : 0))) + tapfdName, tapfdSize, + vhostfdName, vhostfdSize))) goto cleanup; } else { if (!(netstr = qemuBuildHostNetStr(net, driver, priv-qemuCaps, ' ', vlan, - tapfd_name, tapfd_name ? 1 : 0, - vhostfd_name, vhostfd_name ? 1 : 0))) +
[libvirt] [PATCH v1 11/12] qemu: Adapt qemuBuildInterfaceCommandLine to to multiqueue net
--- src/qemu/qemu_command.c | 88 +++-- 1 file changed, 64 insertions(+), 24 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index dfcfedc..60873c7 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -5816,12 +5816,16 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, enum virNetDevVPortProfileOp vmop) { int ret = -1; -int tapfd = -1; char *nic = NULL, *host = NULL; -char *tapfdName = NULL; -char *vhostfdName = NULL; +int *tapfd = NULL; +int tapfdSize = 0; +int *vhostfd = NULL; +int vhostfdSize = 0; +char **tapfdName = NULL; +char **vhostfdName = NULL; int bootindex = bootNet; int actualType; +int i; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); if (!bootindex) @@ -5883,14 +5887,25 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, if (actualType == VIR_DOMAIN_NET_TYPE_NETWORK || cfg-privileged || (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV_BRIDGE))) { +if (VIR_ALLOC(tapfd) 0 || VIR_ALLOC(tapfdName) 0) { +virReportOOMError(); +goto cleanup; +} + +tapfdSize = 1; if (qemuNetworkIfaceConnect(def, conn, driver, net, -qemuCaps, tapfd, 1) 0) +qemuCaps, tapfd, tapfdSize) 0) goto cleanup; } } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { -tapfd = qemuPhysIfaceConnect(def, driver, net, - qemuCaps, vmop); -if (tapfd 0) +if (VIR_ALLOC(tapfd) 0 || VIR_ALLOC(tapfdName) 0) { +virReportOOMError(); +goto cleanup; +} +tapfdSize = 1; +tapfd[0] = qemuPhysIfaceConnect(def, driver, net, +qemuCaps, vmop); +if (tapfd[0] 0) goto cleanup; } @@ -5898,31 +5913,42 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, actualType == VIR_DOMAIN_NET_TYPE_BRIDGE || actualType == VIR_DOMAIN_NET_TYPE_ETHERNET || actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { +int useVhost; /* Attempt to use vhost-net mode for these types of network device */ -int vhostfd; - -if (qemuOpenVhostNet(def, net, qemuCaps, vhostfd, 1) 0) +if (VIR_ALLOC(vhostfd) 0 || VIR_ALLOC(vhostfdName)) { +virReportOOMError(); goto cleanup; -if (vhostfd = 0) { -virCommandTransferFD(cmd, vhostfd); - -if (virAsprintf(vhostfdName, %d, vhostfd) 0) { -virReportOOMError(); -goto cleanup; -} } +vhostfdSize = 1; + +useVhost = qemuOpenVhostNet(def, net, qemuCaps, vhostfd, vhostfdSize); +if (useVhost 0) +goto cleanup; +if (useVhost 0) +vhostfdSize = 0; } -if (tapfd = 0) { -virCommandTransferFD(cmd, tapfd); +for (i = 0; i tapfdSize; i++) { +virCommandTransferFD(cmd, tapfd[i]); -if (virAsprintf(tapfdName, %d, tapfd) 0) { +if (virAsprintf(tapfdName[i], %d, tapfd[i]) 0) { virReportOOMError(); goto cleanup; } } +for (i = 0; i vhostfdSize; i++) { +if (vhostfd[i] = 0) { +virCommandTransferFD(cmd, vhostfd[i]); + +if (virAsprintf(vhostfdName[i], %d, vhostfd[i]) 0) { +virReportOOMError(); +goto cleanup; +} +} +} + /* Possible combinations: * * 1. Old way: -net nic,model=e1000,vlan=1 -net tap,vlan=1 @@ -5935,8 +5961,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps, ',', vlan, - tapfdName, tapfdName ? 1 : 0, - vhostfdName, vhostfdName ? 1 : 0))) + tapfdName, tapfdSize, + vhostfdName, vhostfdSize))) goto cleanup; virCommandAddArgList(cmd, -netdev, host, NULL); } @@ -5953,8 +5979,8 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) { if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps, ',', vlan, - tapfdName, tapfdName ? 1 : 0, - vhostfdName, vhostfdName ? 1 : 0))) + tapfdName, tapfdSize, + vhostfdName, vhostfdSize)))
[libvirt] [PATCH v1 05/12] qemu: Adapt command line generation to multiqueue net
The qemuBuildHostNetStr() function which is responsible for generating command line for a network interface needs to be aware of multiqueue network interface as we are required to use: - fd=%d in case of one FD - fds=%d:%d:%d:...:%d in case of multiple FDs These two approaches can't be mixed. Same applies for vhost FDs. --- src/qemu/qemu_command.c | 46 +- src/qemu/qemu_command.h | 6 -- src/qemu/qemu_hotplug.c | 10 ++ 3 files changed, 47 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 4af2d04..b5374e0 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3748,14 +3748,17 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, char type_sep, int vlan, -const char *tapfd, -const char *vhostfd) +char **tapfd, +int tapfdSize, +char **vhostfd, +int vhostfdSize) { bool is_tap = false; virBuffer buf = VIR_BUFFER_INITIALIZER; enum virDomainNetType netType = virDomainNetGetActualType(net); const char *brname = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); +int i; if (net-script netType != VIR_DOMAIN_NET_TYPE_ETHERNET) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, @@ -3782,7 +3785,19 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, } case VIR_DOMAIN_NET_TYPE_NETWORK: case VIR_DOMAIN_NET_TYPE_DIRECT: -virBufferAsprintf(buf, tap%cfd=%s, type_sep, tapfd); +virBufferAsprintf(buf, tap%c, type_sep); +/* for one tapfd 'fd=' shall be used, + * for more than one 'fds=' is the right choice */ +if (tapfdSize == 1) { +virBufferAsprintf(buf, fd=%s, tapfd[0]); +} else { +virBufferAddLit(buf, fds=); +for (i = 0; i tapfdSize; i++) { +if (i) +virBufferAddChar(buf, ':'); +virBufferAdd(buf, tapfd[i], -1); +} +} type_sep = ','; is_tap = true; break; @@ -3842,8 +3857,19 @@ qemuBuildHostNetStr(virDomainNetDefPtr net, } if (is_tap) { -if (vhostfd *vhostfd) -virBufferAsprintf(buf, ,vhost=on,vhostfd=%s, vhostfd); +if (vhostfdSize) { +virBufferAddLit(buf, ,vhost=on,); +if (vhostfdSize == 1) { +virBufferAsprintf(buf, vhostfd=%s, vhostfd[0]); +} else { +virBufferAddLit(buf, vhostfds=); +for (i = 0; i vhostfdSize; i++) { +if (i) +virBufferAddChar(buf, ':'); +virBufferAdd(buf, vhostfd[i], -1); +} +} +} if (net-tune.sndbuf_specified) virBufferAsprintf(buf, ,sndbuf=%lu, net-tune.sndbuf); } @@ -5882,8 +5908,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps, - ',', vlan, tapfdName, - vhostfdName))) + ',', vlan, + tapfdName, tapfdName ? 1 : 0, + vhostfdName, vhostfdName ? 1 : 0))) goto cleanup; virCommandAddArgList(cmd, -netdev, host, NULL); } @@ -5899,8 +5926,9 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, if (!(virQEMUCapsGet(qemuCaps, QEMU_CAPS_NETDEV) virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE))) { if (!(host = qemuBuildHostNetStr(net, driver, qemuCaps, - ',', vlan, tapfdName, - vhostfdName))) + ',', vlan, + tapfdName, tapfdName ? 1 : 0, + vhostfdName, vhostfdName ? 1 : 0))) goto cleanup; virCommandAddArgList(cmd, -net, host, NULL); } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index 1789c20..d1ae325 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -74,8 +74,10 @@ char * qemuBuildHostNetStr(virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, char type_sep, int vlan, - const char *tapfd, - const char *vhostfd); + char **tapfd, + int tapfdSize, + char **vhostfd, + int vhostfdSize);
[libvirt] [PATCH v1 10/12] qemu: Change qemuOpenVhostNet return value
Currently, need for use of vhost-net is signalized by returning zero, and setting passed FD to a value different to negative one. However, when using multiple vhost-net devices, it is not so easy so we should use return value for that. --- src/qemu/qemu_command.c | 22 +- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 58c81d7..dfcfedc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -320,6 +320,19 @@ cleanup: } +/** + * qemuOpenVhostNet: + * @def: domain definition + * @net: network definition + * @qemuCaps: qemu binary capabilities + * @vhostfd: array of opened vhost-net device + * @vhostfdSize: size of @vhostfd array + * + * Open vhost-net, multiple times - if request. + * Returns: 1 if no vhost-net is required for @net type + * 0 on success + * -1 on failure + */ int qemuOpenVhostNet(virDomainDefPtr def, virDomainNetDefPtr net, @@ -333,9 +346,8 @@ qemuOpenVhostNet(virDomainDefPtr def, vhostfd[i] = -1; /* assume we won't use vhost */ /* If the config says explicitly to not use vhost, return now */ -if (net-driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) { - return 0; -} +if (net-driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) + return 1; /* If qemu doesn't support vhost-net mode (including the -netdev command * option), don't try to open the device. @@ -349,7 +361,7 @@ qemuOpenVhostNet(virDomainDefPtr def, this QEMU binary)); return -1; } -return 0; +return 1; } /* If the nic model isn't virtio, don't try to open. */ @@ -360,7 +372,7 @@ qemuOpenVhostNet(virDomainDefPtr def, virtio network interfaces)); return -1; } -return 0; +return 1; } for (i = 0; i vhostfdSize; i++) { -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 00/12] Multiple TX queue support
Kernel and subsequently QEMU learned multiple transmit queues a while ago. The feature has a nice advantage, it alloes a single guest to transmit multiple flows of network data using multiple CPUs simultaneously which increase traffic bandwidth. A lot. The documentation how to use this is available at [1] or [2]. 1: https://git.kernel.org/cgit/linux/kernel/git/davem/net-next.git/tree/Documentation/networking/tuntap.txt 2: http://git.qemu.org/?p=qemu.git;a=blob;f=qemu-options.hx;hb=HEAD#l1363 Michal Privoznik (12): Introduce /domain/devices/interface/driver/@queues attribute qemu: Move interface cmd line construction into a separate function qemu: Make qemuMonitorAddHostNetwork to pass multiple FDs qemu: Make qemuMonitorAddHostNetwork to pass multiple FDs qemu: Adapt command line generation to multiqueue net util: Learn virNetDevTapCreate multiqueue network qemu: Allow multiple vhost-net openings qemu: Rework qemuNetworkIfaceConnect qemu: Adapt qemuDomainAttachNetDevice to multiqueue net qemu: Change qemuOpenVhostNet return value qemu: Adapt qemuBuildInterfaceCommandLine to to multiqueue net qemu: Enable multiqueue network docs/formatdomain.html.in | 11 +- docs/schemas/domaincommon.rng | 5 + src/conf/domain_conf.c | 15 + src/conf/domain_conf.h | 1 + src/network/bridge_driver.c| 2 +- src/qemu/qemu_command.c| 527 + src/qemu/qemu_command.h| 13 +- src/qemu/qemu_domain.c | 27 +- src/qemu/qemu_hotplug.c| 99 ++-- src/qemu/qemu_monitor.c| 78 +-- src/qemu/qemu_monitor.h| 8 +- src/uml/uml_conf.c | 5 +- src/util/virnetdevtap.c| 105 ++-- src/util/virnetdevtap.h| 2 + tests/qemuxml2argvdata/qemuxml2argv-event_idx.xml | 2 +- .../qemuxml2argv-net-virtio-device.xml | 2 +- .../qemuxml2argvdata/qemuxml2argv-vhost_queues.xml | 50 ++ tests/qemuxml2argvdata/qemuxml2argv-virtio-lun.xml | 2 +- tests/qemuxml2xmltest.c| 1 + 19 files changed, 616 insertions(+), 339 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-vhost_queues.xml -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v1 07/12] qemu: Allow multiple vhost-net openings
With multiqueue network feature, we are advised to pass multiple vhost-net FDs as well. The ratio should be 1:1. Therefore we must alter the qemuOpenVhostNet function to allow that. --- src/qemu/qemu_command.c | 40 ++-- src/qemu/qemu_command.h | 3 ++- src/qemu/qemu_hotplug.c | 6 +++--- 3 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f5d11ad..7337069 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -321,9 +321,13 @@ int qemuOpenVhostNet(virDomainDefPtr def, virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, - int *vhostfd) + int *vhostfd, + int vhostfdSize) { -*vhostfd = -1; /* assume we won't use vhost */ +int i; + +for (i = 0; i vhostfdSize; i++) +vhostfd[i] = -1; /* assume we won't use vhost */ /* If the config says explicitly to not use vhost, return now */ if (net-driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_QEMU) { @@ -356,20 +360,28 @@ qemuOpenVhostNet(virDomainDefPtr def, return 0; } -*vhostfd = open(/dev/vhost-net, O_RDWR); -virDomainAuditNetDevice(def, net, /dev/vhost-net, *vhostfd = 0); +for (i = 0; i vhostfdSize; i++) { +vhostfd[i] = open(/dev/vhost-net, O_RDWR); +virDomainAuditNetDevice(def, net, /dev/vhost-net, vhostfd[i] = 0); -/* If the config says explicitly to use vhost and we couldn't open it, - * report an error. - */ -if ((*vhostfd 0) -(net-driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - %s, _(vhost-net was requested for an interface, - but is unavailable)); -return -1; +/* If the config says explicitly to use vhost and we couldn't open it, + * report an error. + */ +if (vhostfd[i] 0 +net-driver.virtio.name == VIR_DOMAIN_NET_BACKEND_TYPE_VHOST) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + %s, _(vhost-net was requested for an interface, + but is unavailable)); +goto error; +} } return 0; + +error: +for (i = 0; i vhostfdSize; i++) +VIR_FORCE_CLOSE(vhostfd[i]); + +return -1; } @@ -5876,7 +5888,7 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd, network device */ int vhostfd; -if (qemuOpenVhostNet(def, net, qemuCaps, vhostfd) 0) +if (qemuOpenVhostNet(def, net, qemuCaps, vhostfd, 1) 0) goto cleanup; if (vhostfd = 0) { virCommandTransferFD(cmd, vhostfd); diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index d1ae325..db14d8f 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -162,7 +162,8 @@ int qemuPhysIfaceConnect(virDomainDefPtr def, int qemuOpenVhostNet(virDomainDefPtr def, virDomainNetDefPtr net, virQEMUCapsPtr qemuCaps, - int *vhostfd); + int *vhostfd, + int vhostfdSize); /* * NB: def-name can be NULL upon return and the caller diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 26b91bc..b34160a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -744,7 +744,7 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, priv-qemuCaps)) 0) goto cleanup; iface_connected = true; -if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd) 0) +if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd, 1) 0) goto cleanup; } } else if (actualType == VIR_DOMAIN_NET_TYPE_DIRECT) { @@ -753,10 +753,10 @@ int qemuDomainAttachNetDevice(virConnectPtr conn, VIR_NETDEV_VPORT_PROFILE_OP_CREATE)) 0) goto cleanup; iface_connected = true; -if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd) 0) +if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd, 1) 0) goto cleanup; } else if (actualType == VIR_DOMAIN_NET_TYPE_ETHERNET) { -if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd) 0) +if (qemuOpenVhostNet(vm-def, net, priv-qemuCaps, vhostfd, 1) 0) goto cleanup; } -- 1.8.1.5 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpc: message related sizes enlarged
On Mon, Apr 22, 2013 at 04:22:26PM +0200, Viktor Mihajlovski wrote: From: Daniel Hansel daniel.han...@linux.vnet.ibm.com We have seen an issue on s390x platform where domain XMLs larger than 1MB were used. The define command was finished successfully. The dumpxml command was not successful (i.e. could not encode message payload). Enlarged message related sizes (e.g. maximum string size, message size, etc.) to handle larger system configurations used on s390x platform. I'm not against this in general, but before we enlarge this so much there needs to be some code work to make RPC message encoding more efficient. Currently virNetMessageEncodeHeader will allocate VIR_NET_MESSAGE_MAX immediately, regardless of whether the message being encoded acutally needs so much. The problem is that we can't tell ahead of time how much space a message needs. We just can't go on doing this if we're going to have such large max message size. We need to start with small allocs grow if we hit the size limit. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Omitting braces with a single-line body
On 04/22/2013 07:40 AM, harryxiyou wrote: On Mon, Apr 22, 2013 at 9:13 PM, Osier Yang jy...@redhat.com wrote: On 22/04/13 20:33, harryxi...@gmail.com wrote: From: Harry Wei harryxi...@gmail.com After i read libvirt/HACKING file, i find we should s/i/I/, Actually, i wonder what 's/i/l/' means? It's capital 'I', not lower case 'l'. That's shorthand using sed syntax for an in-place replacement you should perform on your text. It means you should have written: After I read libvirt/HACKING file, I find we should for that line, because in English, the pronoun 'I' is always capitalized. ACK Thanks ;-) I'll go ahead and touch up the commit message as suggested, and push. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpc: message related sizes enlarged
On 22.04.2013 16:22, Viktor Mihajlovski wrote: From: Daniel Hansel daniel.han...@linux.vnet.ibm.com We have seen an issue on s390x platform where domain XMLs larger than 1MB were used. The define command was finished successfully. The dumpxml command was not successful (i.e. could not encode message payload). Enlarged message related sizes (e.g. maximum string size, message size, etc.) to handle larger system configurations used on s390x platform. Signed-off-by: Daniel Hansel daniel.han...@linux.vnet.ibm.com Signed-off-by: Viktor Mihajlovski mihaj...@linux.vnet.ibm.com --- src/remote/remote_protocol.x |6 +++--- src/rpc/virnetprotocol.x |6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) I am not hesitant to take this patch in. However, src/libvirt.c needs update as well then. See eb635de1. We should tell users in documentation that we have raised the RPC message limit. Or even better - should the limit be turned into a config option so system admins can decide on the most suitable value themselves? On one hand, we need a bigger messages, obviously. But on the other hand, libvirt is allocating a message for each connection so an evil user can do a DOS attack by spoofing message length headers. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Use camelCase for XML attribute numQueues
On 04/22/2013 10:13 AM, Laine Stump wrote: Of course in this case, we *could* avoid the camelCase vs underscore entirely by just naming the attribute queues instead of numQueues or num_queues (the fact that the attribute is a number makes it pretty obvious that it is the number of queues). And I notice that the patches for supporting multiple queues in interfaces uses driver queues='n'/, so consistency would vote in favor of using the same thing for controller. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rng: tighten up domain controller schema
On 22/04/13 22:00, Laine Stump wrote: On 04/19/2013 11:25 AM, Laine Stump wrote: On 04/19/2013 04:32 AM, Osier Yang wrote: On 18/04/13 19:59, Laine Stump wrote: On 04/18/2013 07:27 AM, Osier Yang wrote: On 18/04/13 19:16, Laine Stump wrote: On 04/18/2013 05:41 AM, Martin Kletzander wrote: On 04/18/2013 11:05 AM, Osier Yang wrote: On 18/04/13 17:00, Martin Kletzander wrote: On 04/18/2013 10:54 AM, Osier Yang wrote: On 18/04/13 16:42, Martin Kletzander wrote: On 04/18/2013 06:36 AM, Laine Stump wrote: The rng schema for controller had been non-specific about which types of controllers allowed which models, and also allowed the num_queues attribute (since that hasn't been released yet, should we rename it to numQueues?) Since there's still time (the commit with that is v1.0.4-65-gd4bf0a9), I think we should rename it ASAP since we are using camelCase for all the attribute names. Apart from that, the RNG with this patch is precise according to the documentation, so ACK. I'll try to send the numQueues patch to see what others think. I guess you mean multiple queues support for virtio network? Regardless of which style we will use finally, FYI, num_queues is used for disk.. Personally I'm fine with either, because we already use both across. Yes, I meant the virtio-scsi num_queues. As we're trying not to use underscores in XML, I hope we can still switch it. I haven't found any other num_queues anywhere in the code. Could you point me to the commit that uses that? I'm sending the previously discussed patch in the meantime. Except the virtio-scsi num_queues, there is no other tag for multiple queue yet, we will need a patch to support multiple queue for the network, but it's not committed yet. It's fine to convert it now, 1.0.5 is not released yet. But is it deserved to do, we already have many tags with underscore, which can't be changed for back-compat. I believe those attributes [1] were created by mistake, and kept only because of backward compatibility. I'm trying to be open-minded, though, so I'm not forcing my patch in, but seeing it just as a proposal. Others may have different opinions and I'm willing to discuss that. My first feeling, though, was that we should try to keep the same policy for as many of them as possible. OTOH, I've mistaken the underscore with a hyphen when I remembered what Daniel told me about attributes [2]. I had recalled DV saying something about underscores in the names a long time ago, and I recently asked about underscore vs. camelCase, and danpb said the same thing. (Personally I don't have a preference one way or the other, but if we really are trying to avoid them, now is our chance). I'm fine with either keeping it or changing num_queues. For long term consistence, I agreed with having a consistent naming style is nice. In the meantime, in other device types, we've tried to keep backend details like this pushed into a driver subelement when possible, to avoid polluting the main element (e.g. see the driver subelement of interface). Is it worth putting this numQueues attribute in a driver subelement too? Or am I just playing XML God? Not sure if you mean the upcoming numQueues for interface. But for the existing num_queues, it's for the virtio-scsi controller, putting it in driver doesn't reflect the purpose. But isn't it a backend implementation detail of the specific SCSI controller? In interface and disk, information that is specific to a particular backend (and isn't generally applicable to that type of device) is in the driver subelement. This is the QEMU command line for a virtio-scsi disk: (-device virtio-scsi-pci is mapped to virtio-scsi controller in libvirt XML, with num_queues set): ... -device virtio-scsi-pci,id=scsi0,num_queues=8,bus=pci.0,addr=0x3 \ -usb \ -drive file=/dev/HostVG/QEMUGuest1,if=none,id=drive-scsi0-0-0-0 \ -device scsi-disk,bus=scsi0.0,channel=0,scsi-id=0,lun=0,drive=drive-scsi0-0-0-0,id=scsi0-0-0-0 \ /... And this is the QEMU command line for a virtio disk (with event_idx set): ... -drive file=/var/lib/libvirt/images/f14.img,if=none,id=drive-virtio-disk0 \ -device virtio-blk-pci,event_idx=on,scsi=off,bus=pci.0,addr=0x4,drive=drive-virtio-disk0,id=virtio-disk0 \ /... This is the properties the QEMU device scsi-disk supports: % ./x86_64-softmmu/qemu-system-x86_64 -device scsi-disk,? scsi-disk.drive=drive scsi-disk.logical_block_size=blocksize scsi-disk.physical_block_size=blocksize scsi-disk.min_io_size=uint16 scsi-disk.opt_io_size=uint32 scsi-disk.bootindex=int32 scsi-disk.discard_granularity=uint32 scsi-disk.ver=string scsi-disk.serial=string scsi-disk.vendor=string scsi-disk.product=string scsi-disk.removable=on/off scsi-disk.dpofua=on/off scsi-disk.wwn=hex64 scsi-disk.channel=uint32 scsi-disk.scsi-id=uint32 scsi-disk.lun=uint32 And the properties virtio-blk-pci device supports: % ./x86_64-softmmu/qemu-system-x86_64 -device virtio-blk-pci,? virtio-blk-pci.class=hex32 virtio-blk-pci.ioeventfd=on/off
Re: [libvirt] [PATCH] Use camelCase for XML attribute numQueues
On 22/04/13 22:43, Laine Stump wrote: On 04/22/2013 10:13 AM, Laine Stump wrote: Of course in this case, we *could* avoid the camelCase vs underscore entirely by just naming the attribute queues instead of numQueues or num_queues (the fact that the attribute is a number makes it pretty obvious that it is the number of queues). And I notice that the patches for supporting multiple queues in interfaces uses driver queues='n'/, so consistency would vote in favor of using the same thing for controller. I like the 'queues', as it avoids the question whether to use underscore or camelCase.. :-) So I don't mind changing the existing 'num_queues' into 'queues'. Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rng: tighten up domain controller schema
On 04/22/2013 08:00 AM, Laine Stump wrote: On 04/19/2013 11:25 AM, Laine Stump wrote: On 04/18/2013 06:36 AM, Laine Stump wrote: Trimming a rather deep nesting... Okay, I misunderstood what you said - you weren't saying that you had put num_queues in the disk element (obviously - if I was able to retain enough context in my brain to remember the beginning of the thread, I would have known that :-P), but were instead suggesting that I had meant the num_queues should go in the driver subelement of disk. You misunderstood me (so we're even :-). What I was saying was that it should go in the driver subelement of controller. I still stand by that opinion. Yes, I think a driver subelement of controller is a good idea, and like the compromise of 'queues' instead of 'num_queues' as completely avoiding the _ vs. camelCase question (at least for this issue). -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] virCommandProcessIO: hangs on poll()
On 04/19/2013 01:32 AM, wangxiaojun wrote: recent qemu (git resp) with param like: #qemu-kvm -S -no-user-config -nodefaults -nographic -M none -qmp monarg -pidfile pidfile will just hangs forever. no any output or errors . and when libvirt Try to get caps via QMP qemuCaps. it run above command. so virCommandProcessIO call poll() will be hangs forever. i patch the libvirt (git resp) with : diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 50712b0..915edaa 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2377,7 +2377,6 @@ virQEMUCapsInitQMP(virQEMUCapsPtr qemuCaps, virCommandClearCaps(cmd); virCommandSetGID(cmd, runGid); virCommandSetUID(cmd, runUid); - if (virCommandRun(cmd, status) 0) This hunk is unrelated. goto cleanup; diff --git a/src/util/vircommand.c b/src/util/vircommand.c index ac56a63..84738f9 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -1824,6 +1824,7 @@ virCommandProcessIO(virCommandPtr cmd) int i; struct pollfd fds[3]; int nfds = 0; +int pfds = 0; if (cmd-inpipe != -1) { fds[nfds].fd = cmd-inpipe; @@ -1846,8 +1847,8 @@ virCommandProcessIO(virCommandPtr cmd) if (nfds == 0) break; - -if (poll(fds, nfds, -1) 0) { +pfds = poll(fds, nfds, -1); +if ( pfds 0) { if (errno == EAGAIN || errno == EINTR) continue; virReportSystemError(errno, %s, @@ -1855,10 +1856,22 @@ virCommandProcessIO(virCommandPtr cmd) goto cleanup; } -for (i = 0; i nfds ; i++) { -if (fds[i].revents (POLLIN | POLLHUP | POLLERR) +for (i = 0; i pfds ; i++) { We are guaranteed that pfds = nfds; but it should not hurt to loop over nfds instead of pfds. Furthermore, there is no guarantee that when 'pfds nfds' that the fds that were changed were the first pfds consecutive entries, so looping over nfds actually feels better here. +if (fds[i].revents (POLLHUP | POLLRDHUP | POLLERR | POLLNVAL)) Hmm, you are adding POLLNVAL as a possible flag to look for. +{ +if (fds[i].fd == errfd) { +VIR_DEBUG(hangup on stderr); +} else if (fds[i].fd == outfd) { +VIR_DEBUG(hangup on stdout); +} else { +VIR_DEBUG(hangup on stdin); +} +errfd = -1; +} Why are you blindly changing errfd, even if you checked that it was some other fd that you gave a message about? + +if (fds[i].revents (POLLIN | POLLHUP | POLLRDHUP | POLLERR | POLLNVAL) (fds[i].fd == errfd || fds[i].fd == outfd)) { -char data[1024]; +char data[1024*2]; Why do we need the larger buffer? char **buf; size_t *len; int done; So everythings is OK. i use gentoo OS. qemu git source and libvirt git source. You may be right that our use of poll() isn't entirely correct (it wouldn't be the first time we've botched it, since that interface is notoriously tricky), but if we fix it, I'd like to fix it right, and audit our other uses of poll to make sure they are also right. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH-v4 1/2] create virSocketAddrGetIpPrefix utility function
On 04/22/2013 10:20 AM, Laine Stump wrote: On 04/20/2013 03:45 PM, Gene Czarcinski wrote: Create the utility function virSocketAddrGetIpPrefix() to determine the prefix for this network. The code in this function was adapted from virNetworkIpDefPrefix(). Update virNetworkIpDefPrefix() in src/conf/network_conf.c to use the new utility function. . Signed-off-by: Gene Czarcinski g...@czarc.net --- ACK. I'll push it after I've reviewed 2/2. After going through patch 2/2, I realized that maintaining the return -1 when no prefix or netmask or address is specified rule in this new function will prevent someone from declaring a default route on a network: route gateway='1.2.3.4'/ because this will return -1. I checked all the callers of the original function, and they all are already guaranteed to have a valid address, so we can safely change the default case of this function to return 0. I'm going to make that change and push this patch so that you don't have to resubmit it with the new spin of 2/2 that I'm requesting. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpc: message related sizes enlarged
On 04/22/2013 04:34 PM, Daniel P. Berrange wrote: On Mon, Apr 22, 2013 at 04:22:26PM +0200, Viktor Mihajlovski wrote: I'm not against this in general, but before we enlarge this so much there needs to be some code work to make RPC message encoding more efficient. Currently virNetMessageEncodeHeader will allocate VIR_NET_MESSAGE_MAX immediately, regardless of whether the message being encoded acutally needs so much. The problem is that we can't tell ahead of time how much space a message needs. We just can't go on doing this if we're going to have such large max message size. We need to start with small allocs grow if we hit the size limit. So something along the lines of allocating VIR_NET_MESSAGE_INITIAL bytes, maybe well below 1M, and then bumping it in the EncodePayloadxxx calls if required. Do you foresee any pitfalls one should consider? Daniel -- Mit freundlichen Grüßen/Kind Regards Viktor Mihajlovski IBM Deutschland Research Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Replace more cases of /system with /machine
From: Daniel P. Berrange berra...@redhat.com The change in commit aed4986322fe77bdf718e31a0587d00f04f3d97a was incomplete, missing a couple of cases of /system. This caused failure to start VMs. Pushed under trivial rule Signed-off-by: Daniel P. Berrange berra...@redhat.com --- src/lxc/lxc_cgroup.c | 2 +- src/qemu/qemu_cgroup.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lxc/lxc_cgroup.c b/src/lxc/lxc_cgroup.c index 7311489..4a661da 100644 --- a/src/lxc/lxc_cgroup.c +++ b/src/lxc/lxc_cgroup.c @@ -557,7 +557,7 @@ virCgroupPtr virLXCCgroupCreate(virDomainDefPtr def, bool startup) /* We only auto-create the default partition. In other * cases we expec the sysadmin/app to have done so */ rc = virCgroupNewPartition(def-resource-partition, - STREQ(def-resource-partition, /system), + STREQ(def-resource-partition, /machine), -1, parent); if (rc != 0) { diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 3a58f24..891984a 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -256,7 +256,7 @@ int qemuInitCgroup(virQEMUDriverPtr driver, /* We only auto-create the default partition. In other * cases we expec the sysadmin/app to have done so */ rc = virCgroupNewPartition(vm-def-resource-partition, - STREQ(vm-def-resource-partition, /system), + STREQ(vm-def-resource-partition, /machine), cfg-cgroupControllers, parent); if (rc != 0) { -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpc: message related sizes enlarged
On Mon, Apr 22, 2013 at 06:18:04PM +0200, Viktor Mihajlovski wrote: On 04/22/2013 04:34 PM, Daniel P. Berrange wrote: On Mon, Apr 22, 2013 at 04:22:26PM +0200, Viktor Mihajlovski wrote: I'm not against this in general, but before we enlarge this so much there needs to be some code work to make RPC message encoding more efficient. Currently virNetMessageEncodeHeader will allocate VIR_NET_MESSAGE_MAX immediately, regardless of whether the message being encoded acutally needs so much. The problem is that we can't tell ahead of time how much space a message needs. We just can't go on doing this if we're going to have such large max message size. We need to start with small allocs grow if we hit the size limit. So something along the lines of allocating VIR_NET_MESSAGE_INITIAL bytes, maybe well below 1M, and then bumping it in the EncodePayloadxxx calls if required. Do you foresee any pitfalls one should consider? We have an ever increasing variability in RPC message size - some of them will remain absolutely tiny - 24 bytes in size, while others can be huge. Deciding on the optimal initial allocation size is a tradeoff between the overheads of frequently re-alloc'ing to enlarge the buffer, vs the overhead of allocting too much memory and/or calling memset() on the buffers. I'd suggest an initial size of just a perhaps as small as a few 10's of KB ? Then do a loop doubling allocation each time until it hits the hard limit. In cases where we even up triggering the re-alloc loop many times, we'll presumably be on large hardware with many many VMs, so the overhead of the re-allocs will be dwarved by all the other activity on the machine. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/3] qemu: Ignore libvirt logs when reading QEMU error output
When QEMU fails to start, libvirt read its error output and reports it back in an error message. However, when libvirtd is configured to log debug messages, one would get the following unhelpful garbage: virsh # start cd error: Failed to start domain cd error: internal error process exited while connecting to monitor: \ 2013-04-22 14:24:54.214+: 2194219: debug : virFileClose:72 : \ Closed fd 21 2013-04-22 14:24:54.214+: 2194219: debug : virFileClose:72 : \ Closed fd 27 2013-04-22 14:24:54.215+: 2194219: debug : virFileClose:72 : \ Closed fd 3 2013-04-22 14:24:54.215+: 2194220: debug : virExec:602 : Run \ hook 0x7feb8f600bf0 0x7feb86ef9300 2013-04-22 14:24:54.215+: 2194220: debug : qemuProcessHook:2507 \ : Obtaining domain lock 2013-04-22 14:24:54.216+: 2194220: debug : \ virDomainLockProcessStart:170 : plugin=0x7feb780261f0 \ dom=0x7feb7802a360 paused=1 fd=0x7feb86ef8ec4 2013-04-22 14:24:54.216+: 2194220: debug : \ virDomainLockManagerNew:128 : plugin=0x7feb780261f0 \ dom=0x7feb7802a360 withResources=1 2013-04-22 14:24:54.216+: 2194220: debug : \ virLockManagerPluginGetDriver:297 : plugin=0x7feb780261f0 2013-04-22 14:24:54.216+: 2194220: debug : \ virLockManagerNew:321 : driver=0x7feb8ef08640 type=0 nparams=5 \ params=0x7feb86ef8d60 flags=0 2013-04-22 14:24:54.216+000 instead of (the output with this patch applied): virsh # start cd error: Reconnected to the hypervisor error: Failed to start domain cd error: internal error process exited while connecting to monitor: \ char device redirected to /dev/pts/33 (label charserial0) qemu-system-x86_64: -drive file=/home/vm/systemrescuecd-x86-1.2.0.\ iso,if=none,id=drive-ide0-1-0,readonly=on,format=raw,cache=none: \ could not open disk image /home/vm/systemrescuecd-x86-1.2.0.iso: \ Permission denied --- src/qemu/qemu_process.c | 20 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a1fbd5f..4988d9b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1665,21 +1665,6 @@ static void qemuProcessFreePtyPath(void *payload, const void *name ATTRIBUTE_UNU VIR_FREE(payload); } -static void -qemuProcessReadLogFD(int logfd, char *buf, int maxlen, int off) -{ -int ret; -char *tmpbuf = buf + off; - -ret = saferead(logfd, tmpbuf, maxlen - off - 1); -if (ret 0) { -ret = 0; -} - -tmpbuf[ret] = '\0'; -} - - static int qemuProcessWaitForMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -1734,6 +1719,7 @@ cleanup: virHashFree(paths); if (pos != -1 kill(vm-pid, 0) == -1 errno == ESRCH) { +int len; /* VM is dead, any other error raised in the interim is probably * not as important as the qemu cmdline output */ if (virQEMUCapsUsedQMP(qemuCaps)) { @@ -1745,7 +1731,9 @@ cleanup: goto closelog; } } -qemuProcessReadLogFD(logfd, buf, buf_size, strlen(buf)); + +len = strlen(buf); +qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0); virReportError(VIR_ERR_INTERNAL_ERROR, _(process exited while connecting to monitor: %s), buf); -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/3] logging: Make log regexp more compact (and readable)
--- src/util/virlog.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 721c9bd..e31abb0 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -79,8 +79,8 @@ static int virLogEnd = 0; static regex_t *virLogRegex = NULL; -#define VIR_LOG_DATE_REGEX [0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9] -#define VIR_LOG_TIME_REGEX [0-9][0-9]:[0-9][0-9]:[0-9][0-9]\\.[0-9][0-9][0-9]\\+[0-9][0-9][0-9][0-9] +#define VIR_LOG_DATE_REGEX [0-9]{4}-[0-9]{2}-[0-9]{2} +#define VIR_LOG_TIME_REGEX [0-9]{2}:[0-9]{2}:[0-9]{2}\\.[0-9]{3}\\+[0-9]{4} #define VIR_LOG_PID_REGEX [0-9]+ #define VIR_LOG_LEVEL_REGEX debug|info|warning|error -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 2/3] Only create the destination path if it does not exist.
From: Dan Walsh dwa...@redhat.com OpenShift will be creating the path within its management layer. --- bin/virt-sandbox-service | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service index 31aa6a1..dd30993 100755 --- a/bin/virt-sandbox-service +++ b/bin/virt-sandbox-service @@ -272,7 +272,8 @@ class Container: def create(self): self.connect() self.config.set_shell(True) -os.mkdir(self.dest) +if not os.path.exists(self.dest): +os.mkdir(self.dest) def connect(self): if not self.conn: @@ -331,6 +332,9 @@ class GenericContainer(Container): self.save_config() def create(self): +config_path = self.get_config_path() +if os.path.exists(config_path): +raise ValueError([_(%s already exists) % config_path ]) try: self.create_generic() except Exception, e: -- 1.8.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 3/3] Do not run a shell within a lxc container by default.
From: Dan Walsh dwa...@redhat.com We want to make sure we use as little overhead as possible. If a user connects to a lxc container, it will be the same as executing a shell within the container. --- bin/virt-sandbox-service | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service index dd30993..4d2d1cd 100755 --- a/bin/virt-sandbox-service +++ b/bin/virt-sandbox-service @@ -271,7 +271,8 @@ class Container: def create(self): self.connect() -self.config.set_shell(True) +if self.uri != lxc:///: +self.config.set_shell(True) if not os.path.exists(self.dest): os.mkdir(self.dest) @@ -843,6 +844,18 @@ def stop(args): -S, args.name) def connect(args): +if args.uri == lxc:///: +class Args: +command = [] +noseclabel = None +name = args.name +uri = args.uri + +args = Args() +args.command = [ /bin/sh ] +execute(args) +return + print \ Connected to %s. Type 'Ctrl + ]' to detach from the console. -- 1.8.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Add UID/GID support to virt-sandbox-service
This patch set is adding support for UID/GID/USERNAME/USERDIR for use with openshift containers Also fixes virt-sandbox-service to not complain if the destdir has been precreated. Finally we also do not want excess processes running withing containers (/bin/sh). [sandbox PATCH 1/3] Add UID/GID support for use with interactive [sandbox PATCH 2/3] Only create the destination path if it does not [sandbox PATCH 3/3] Do not run a shell within a lxc container by -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 1/3] Add UID/GID support for use with interactive containers.
On Mon, Apr 22, 2013 at 12:26:29PM -0400, dwa...@redhat.com wrote: From: Dan Walsh dwa...@redhat.com Openshift Containers will be run with a unique UID and GID --- bin/virt-sandbox-service| 43 +-- bin/virt-sandbox-service-bash-completion.sh | 8 +++-- bin/virt-sandbox-service-create.pod | 53 - 3 files changed, 90 insertions(+), 14 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 0/3] Ignore libvirt logs when reading QEMU error output
Jiri Denemark (3): qemu: Move QEMU log reading into a separate function qemu: Ignore libvirt logs when reading QEMU error output logging: Make log regexp more compact (and readable) src/qemu/qemu_process.c | 81 + src/util/virlog.c | 4 +-- 2 files changed, 44 insertions(+), 41 deletions(-) -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 2/3] Only create the destination path if it does not exist.
On Mon, Apr 22, 2013 at 12:26:30PM -0400, dwa...@redhat.com wrote: From: Dan Walsh dwa...@redhat.com OpenShift will be creating the path within its management layer. --- bin/virt-sandbox-service | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service index 31aa6a1..dd30993 100755 --- a/bin/virt-sandbox-service +++ b/bin/virt-sandbox-service @@ -272,7 +272,8 @@ class Container: def create(self): self.connect() self.config.set_shell(True) -os.mkdir(self.dest) +if not os.path.exists(self.dest): +os.mkdir(self.dest) def connect(self): if not self.conn: ACK to this chunk. @@ -331,6 +332,9 @@ class GenericContainer(Container): self.save_config() def create(self): +config_path = self.get_config_path() +if os.path.exists(config_path): +raise ValueError([_(%s already exists) % config_path ]) This appears to be unrelated to the subject of this patch. Please move it to a separate commit. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [sandbox PATCH 3/3] Do not run a shell within a lxc container by default.
On Mon, Apr 22, 2013 at 12:26:31PM -0400, dwa...@redhat.com wrote: From: Dan Walsh dwa...@redhat.com We want to make sure we use as little overhead as possible. If a user connects to a lxc container, it will be the same as executing a shell within the container. --- bin/virt-sandbox-service | 15 ++- 1 file changed, 14 insertions(+), 1 deletion(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [sandbox PATCH 1/3] Add UID/GID support for use with interactive containers.
From: Dan Walsh dwa...@redhat.com Openshift Containers will be run with a unique UID and GID --- bin/virt-sandbox-service| 43 +-- bin/virt-sandbox-service-bash-completion.sh | 8 +++-- bin/virt-sandbox-service-create.pod | 53 - 3 files changed, 90 insertions(+), 14 deletions(-) diff --git a/bin/virt-sandbox-service b/bin/virt-sandbox-service index a99fe7e..31aa6a1 100755 --- a/bin/virt-sandbox-service +++ b/bin/virt-sandbox-service @@ -28,6 +28,7 @@ import exceptions import rpm from subprocess import Popen, PIPE, STDOUT import gettext +import pwd if os.path.exists(/sys/fs/selinux): import selinux @@ -83,10 +84,28 @@ class Container: self.file_type = file_type def set_uid(self, uid): -self.uid = uid +self.config.set_userid(uid) def get_uid(self): -return self.uid +return self.config.get_userid(uid) + +def set_gid(self, gid): +self.config.set_groupid(gid) + +def get_gid(self): +return self.config.get_groupid(gid) + +def set_username(self, username): +self.config.set_username(username) + +def get_username(self): +return self.config.get_username() + +def set_homedir(self, homedir): +self.config.set_homedir(homedir) + +def get_homedir(self): +return self.config.get_homedir() def get_config_path(self, name = None): if not name: @@ -755,6 +774,16 @@ def create(args): container.add_network(net) if args.security: container.set_security(args.security) +container.set_uid(args.uid) +if not args.homedir: +args.homedir = pwd.getpwuid(args.uid).pw_dir +container.set_homedir(args.homedir) +if not args.username: +args.username = pwd.getpwuid(args.uid).pw_name +container.set_username(args.username) +if not args.gid: +args.gid = pwd.getpwuid(args.uid).pw_gid +container.set_gid(args.gid) container.set_path(args.path) container.set_file_type(args.file_type) if args.imagesize: @@ -952,6 +981,11 @@ def gen_create_args(subparser): parser.add_argument(-f, --filetype, dest=file_type, default=c.get_file_type(), help=_(SELinux file type to assign to content within the sandbox. Default: %s) % c.get_file_type()) +parser.add_argument(--homedir, dest=homedir, +help=_(Specify the homedir for the container. Default: UID homedir.)) +parser.add_argument(-G, --gid, dest=gid, +default=None, type=int, +help=_(Specify the login gid for the container. Default: login GID of the UID.)) parser.add_argument(-i, --imagesize, dest=imagesize, default = None, action=SizeAction, help=_(create image of this many megabytes.)) @@ -967,6 +1001,11 @@ def gen_create_args(subparser): action=CheckUnit, dest=unitfiles, default=[], help=_(Systemd Unit file to run within the systemd sandbox container. Commands cannot be specified with unit files.)) +parser.add_argument(--username, dest=username, +help=_(Specify the username for the container. Default: UID username.)) +parser.add_argument(-U, --uid, dest=uid, +default=os.getuid(),type=int, +help=_(Specify the uid for the container: Default to current UID.)) requires_name(parser) parser.add_argument(command, default=[], nargs=*, diff --git a/bin/virt-sandbox-service-bash-completion.sh b/bin/virt-sandbox-service-bash-completion.sh index 874ee56..8f2b6d0 100755 --- a/bin/virt-sandbox-service-bash-completion.sh +++ b/bin/virt-sandbox-service-bash-completion.sh @@ -1,6 +1,6 @@ # This file is part of libvirt-sandbox. # -# Copyright 2012 Dan Walsh +# Copyright (C) 2012-2013 Red Hat, Inc. # # systemd is free software; you can redistribute it and/or modify it # under the terms of the GNU General Public License as published by @@ -14,7 +14,9 @@ # # You should have received a copy of the GNU General Public License # along with systemd; If not, see http://www.gnu.org/licenses/. - +# +# Authors: Dan Walsh dwa...@redhat.com +# __contains_word () { local word=$1; shift for w in $*; do [[ $w = $word ]] return 0; done @@ -55,7 +57,7 @@ _virt_sandbox_service () { ) local -A OPTS=( [ALL]='-h --help' -[CREATE]='-u --unitfile -p --path -f --filetype -C --copy -i --imagesize -N --network -s --security' +[CREATE]='-C --copy -f --filetype -G --gid -i --imagesize --homedir -N --network -p --path -s --security -u --unitfile --username -U -uid' [LIST]='-r --running' [RELOAD]='-u --unitfile' [EXECUTE]='-N --noseclabel' diff --git a/bin/virt-sandbox-service-create.pod
[libvirt] [PATCH 1/3] qemu: Move QEMU log reading into a separate function
--- src/qemu/qemu_process.c | 61 ++--- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ce9f501..a1fbd5f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1346,6 +1346,41 @@ error: return ret; } +static int +qemuProcessReadLog(int fd, char *buf, int buflen, int off) +{ +char *filter_next = buf; +ssize_t bytes; +char *eol; + +buf[0] = '\0'; + +while (off buflen - 1) { +bytes = saferead(fd, buf + off, buflen - off - 1); +if (bytes 0) +return -1; +else if (bytes == 0) +break; + +off += bytes; +buf[off] = '\0'; + +/* Filter out debug messages from intermediate libvirt process */ +while ((eol = strchr(filter_next, '\n'))) { +*eol = '\0'; +if (virLogProbablyLogMessage(filter_next)) { +memmove(filter_next, eol + 1, off - (eol - buf)); +off -= eol + 1 - filter_next; +} else { +filter_next = eol + 1; +*eol = '\n'; +} +} +} + +return off; +} + typedef int qemuProcessLogHandleOutput(virDomainObjPtr vm, const char *output, int fd); @@ -1365,46 +1400,26 @@ qemuProcessReadLogOutput(virDomainObjPtr vm, int retries = (timeout*10); int got = 0; int ret = -1; -char *filter_next = buf; buf[0] = '\0'; while (retries) { -ssize_t func_ret, bytes; +ssize_t func_ret; int isdead = 0; -char *eol; func_ret = func(vm, buf, fd); if (kill(vm-pid, 0) == -1 errno == ESRCH) isdead = 1; -/* Any failures should be detected before we read the log, so we - * always have something useful to report on failure. */ -bytes = saferead(fd, buf+got, buflen-got-1); -if (bytes 0) { +got = qemuProcessReadLog(fd, buf, buflen, got); +if (got 0) { virReportSystemError(errno, _(Failure while reading %s log output), what); goto cleanup; } -got += bytes; -buf[got] = '\0'; - -/* Filter out debug messages from intermediate libvirt process */ -while ((eol = strchr(filter_next, '\n'))) { -*eol = '\0'; -VIR_ERROR(%s, filter_next); -if (virLogProbablyLogMessage(filter_next)) { -memmove(filter_next, eol + 1, got - (eol - buf)); -got -= eol + 1 - filter_next; -} else { -filter_next = eol + 1; -*eol = '\n'; -} -} - if (got == buflen-1) { virReportError(VIR_ERR_INTERNAL_ERROR, _(Out of space while reading %s log output: %s), -- 1.8.2.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] Configure native vlan modes on Open vSwitch ports
Date: Thu, 18 Apr 2013 14:14:32 -0400 From: Laine Stump la...@laine.org To: libvir-list@redhat.com Subject: Re: [libvirt] [PATCHv2] Configure native vlan modes on Open vSwitch ports Message-ID: 51703808.2000...@laine.org Content-Type: text/plain; charset=ISO-8859-1 On 04/18/2013 01:44 PM, james robson wrote: Hello, Has any one been able to review this yet? I realise that the 'Since 1.0.3' in the doc page is now out of date, but is the code itself acceptable? I was hoping that someone with more knowledge of Open vSwitch and/or vlan tagging/trunking/native mode would repond to the message (Kyle?) but there was silence instead... diff --git a/tests/networkxml2xmlin/openvswitch-net.xml b/tests/networkxml2xmlin/openvswitch-net.xml index a3d82b1..93c49d5 100644 --- a/tests/networkxml2xmlin/openvswitch-net.xml +++ b/tests/networkxml2xmlin/openvswitch-net.xml @@ -21,4 +21,13 @@ parameters profileid='alice-profile'/ /virtualport /portgroup + portgroup name='tagged' +vlan native_mode='tagged' native_tag='123' + tag id='555'/ + tag id='444'/ +/vlan +virtualport + parameters profileid='tagged-profile'/ +/virtualport + /portgroup As brought up again in a separate conversation today, we prefer to use camelCase rather than underscored in attribute and element names. So, if we were to use the layout you're proposing, the attributes should be called nativeMode and nativeTag. However, I'm wondering if there might be a better way to structure it. What about this? vlan trunk='yes' tag id='123' native='tagged|untagged'/ (or whatever values are appropriate) tag id='555'/ tag id='444'/ /vlan Do I understand correctly that native mode is telling what to do with packets that come in untagged, and that (using your nomenclature native_mode='yes' native_tag='123' means when an untagged packet come in from this interface, it should be tagged as 123 before forwarding? That is correct, setting the native vlan changes how an untagged packet is handled when it enters the port. The difference between the 'tagged' and 'untagged' modes is in how packets on the native vlan are processed before exiting the port. And what happens when native_mode='yes' but there is no native_tag? In that case you configuration is invalid, and will get an error. (that's what I was trying to describe with tag id='123' native='untagged'/, but I don't even know if that makes sense, because I don't know exactly what is the native vlan tag and what is done with it :-) That arrangement would make sense, I chose the arrangement I did for two main reasons. There can only be one native vlan on a port, making it an attribute of the 'vlan' tag enforces this. Also, I wanted to keep the validation and processing separate rather than add 'if native' branches to the loops that operate on the vlan id list. I can see the advantage of having a single setting to configure the native vlan, rather than the two attributes I proposed. If the new suggestion is preferred I can rework my patch to use that format. Also, is it valid to have a native_mode/native_tag if trunk='no'? (right now trunk is automatically set to 'yes' if there is more than one vlan tag) It isn't valid to have trunk='no' and the native settings. Therefore vlan trunk='no' native_mode='tagged' native_tag='123' will get an error if you try to enter it. If no trunk attribute is set explicitly then it will be set to 'yes'. This means vlan native_mode='tagged' native_tag='123' is equivalent to vlan trunk='yes' native_mode='tagged' native_tag='123'. /network diff --git a/tests/networkxml2xmlout/openvswitch-net.xml b/tests/networkxml2xmlout/openvswitch-net.xml index a3d82b1..ab3d797 100644 --- a/tests/networkxml2xmlout/openvswitch-net.xml +++ b/tests/networkxml2xmlout/openvswitch-net.xml @@ -21,4 +21,13 @@ parameters profileid='alice-profile'/ /virtualport /portgroup + portgroup name='tagged' +vlan trunk='yes' native_mode='tagged' native_tag='123' + tag id='555'/ + tag id='444'/ +/vlan +virtualport + parameters profileid='tagged-profile'/ +/virtualport + /portgroup /network Protected by Websense Hosted Email Security -- www.websense.com -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix setting of memory tunables
On 04/19/2013 04:01 AM, Peter Krempa wrote: Refactoring done in 19c6ad9ac7e7eb2fd3c8262bff5f087b508ad07f didn't correctly take into account the order cgroup limit modification needs to be done in. This resulted into errors when decreasing the limits. The operations need to take place in this order: decrease hard limit change swap hard limit or change swap hard limit increase hard limit This patch also fixes the check if the hard_limit is less than swap_hard_limit to print better error messages. For this purpose I introduced a helper function virCompareLimitUlong to compare limit values where value of 0 is equal to unlimited. Additionally the check is now applied also when the user does not provide all of the tunables through the API and in that case the currently set values are used. This patch resolves: https://bugzilla.redhat.com/show_bug.cgi?id=950478 --- src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 95 src/util/virutil.c | 12 ++ src/util/virutil.h | 2 + 4 files changed, 63 insertions(+), 47 deletions(-) +++ b/src/util/virutil.c @@ -3832,3 +3832,15 @@ virFindFCHostCapableVport(const char *sysfs_prefix ATTRIBUTE_UNUSED) } #endif /* __linux__ */ + +int +virCompareLimitUlong(unsigned long long a, unsigned long b) Please document this function, and that it special-cases 0 meaning unlimited. ACK with that improved. -- 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] [libvirt-designer 1/3] Fix 'dependancy' typo in README file
--- README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README b/README index ed39b9d..b7ca356 100644 --- a/README +++ b/README @@ -24,7 +24,7 @@ Or to install into a private user specific location make make install -The following mandatory dependancies are required in order to +The following mandatory dependencies are required in order to build libvirt-designer libvirt-gconfig = 0.1.3 (part of libvirt-glib project releases) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer 3/3] Set min versions from configure in .spec file
Minimum libvirt-gobject, libvirt-gconfig and libosinfo versions are set in configure.ac, it's better not to duplicate them in libvirt-designer.spec.in, it's too easy to get them out of sync. --- configure.ac | 2 +- libvirt-designer.spec.in | 8 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 7167cd1..ebb29e7 100644 --- a/configure.ac +++ b/configure.ac @@ -15,9 +15,9 @@ LIBVIRT_GCONFIG_REQUIRED=0.1.7 LIBVIRT_GOBJECT_REQUIRED=0.1.3 GOBJECT_INTROSPECTION_REQUIRED=0.10.8 -# for proper substitution in README.in AC_SUBST(LIBOSINFO_REQUIRED) AC_SUBST(LIBVIRT_GCONFIG_REQUIRED) +AC_SUBST(LIBVIRT_GOBJECT_REQUIRED) LIBVIRT_DESIGNER_MAJOR_VERSION=`echo $VERSION | awk -F. '{print $1}'` LIBVIRT_DESIGNER_MINOR_VERSION=`echo $VERSION | awk -F. '{print $2}'` diff --git a/libvirt-designer.spec.in b/libvirt-designer.spec.in index d8145b4..894bcb8 100644 --- a/libvirt-designer.spec.in +++ b/libvirt-designer.spec.in @@ -20,12 +20,12 @@ License: LGPLv2+ URL: http://libvirt.org/ Source0: http://libvirt.org/sources/designer/%{name}-%{version}.tar.gz BuildRoot: %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n) -BuildRequires: libvirt-gconfig-devel = 0.0.9 -BuildRequires: libvirt-gobject-devel = 0.1.3 +BuildRequires: libvirt-gconfig-devel = @LIBVIRT_GCONFIG_REQUIRED@ +BuildRequires: libvirt-gobject-devel = @LIBVIRT_GOBJECT_REQUIRED@ %if %{with_introspection} BuildRequires: gobject-introspection-devel %endif -BuildRequires: libosinfo-devel = 0.0.5 +BuildRequires: libosinfo-devel = @LIBOSINFO_REQUIRED@ %package libs Group: Development/Libraries @@ -35,7 +35,7 @@ Summary: Libvirt configuration designer libraries Group: Development/Libraries Summary: Libvirt configuration designer development headers Requires: %{name}-libs = %{version}-%{release} -Requires: libvirt-gconfig-devel = 0.0.9 +Requires: libvirt-gconfig-devel = @LIBVIRT_GCONFIG_REQUIRED@ %description This package provides the libvirt configuration designer command -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer 2/3] Automatically set min versions in README
Minimum libvirt-gconfig and libosinfo versions appear in both README and configure.ac, which means they easily get out of sync. This commit renames README to README.in so that we can substitute the configure.ac version in the README file. This way they are always in sync. --- Makefile.am | 1 + README = README.in | 4 ++-- autogen.sh | 4 configure.ac| 5 + 4 files changed, 12 insertions(+), 2 deletions(-) rename README = README.in (93%) diff --git a/Makefile.am b/Makefile.am index b928f83..9ef319a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -10,6 +10,7 @@ EXTRA_DIST = \ $(PACKAGE).spec \ $(pkgconfig_DATA:%.pc=%.pc.in) \ AUTHORS.in \ + README.in \ $(NULL) DISTCLEAN_FILES = $(PACKAGE).spec $(pkgconfig_DATA) diff --git a/README b/README.in similarity index 93% rename from README rename to README.in index b7ca356..2fd8090 100644 --- a/README +++ b/README.in @@ -27,8 +27,8 @@ Or to install into a private user specific location The following mandatory dependencies are required in order to build libvirt-designer - libvirt-gconfig = 0.1.3 (part of libvirt-glib project releases) - libosinfo= 0.0.5 + libvirt-gconfig = @LIBVIRT_GCONFIG_REQUIRED@ (part of libvirt-glib project releases) + libosinfo= @LIBOSINFO_REQUIRED@ Communication diff --git a/autogen.sh b/autogen.sh index 9a0c976..72ce2c0 100755 --- a/autogen.sh +++ b/autogen.sh @@ -41,6 +41,10 @@ fi # exists at all times :-( touch ChangeLog AUTHORS +# README is auto-generated from README.in at configure time, +# but automake requires that it exists at autogen.sh time +touch README + mkdir -p build-aux libtoolize --copy --force aclocal -I m4 diff --git a/configure.ac b/configure.ac index 89128d3..7167cd1 100644 --- a/configure.ac +++ b/configure.ac @@ -15,6 +15,10 @@ LIBVIRT_GCONFIG_REQUIRED=0.1.7 LIBVIRT_GOBJECT_REQUIRED=0.1.3 GOBJECT_INTROSPECTION_REQUIRED=0.10.8 +# for proper substitution in README.in +AC_SUBST(LIBOSINFO_REQUIRED) +AC_SUBST(LIBVIRT_GCONFIG_REQUIRED) + LIBVIRT_DESIGNER_MAJOR_VERSION=`echo $VERSION | awk -F. '{print $1}'` LIBVIRT_DESIGNER_MINOR_VERSION=`echo $VERSION | awk -F. '{print $2}'` LIBVIRT_DESIGNER_MICRO_VERSION=`echo $VERSION | awk -F. '{print $3}'` @@ -145,6 +149,7 @@ fi AM_CONDITIONAL([WITH_VALA], [test x$enable_vala = xyes]) AC_OUTPUT(Makefile + README libvirt-designer/Makefile libvirt-designer.spec libvirt-designer-1.0.pc -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer PATCHv2 2/3] Add gvir_designer_domain_get_supported_devices()
On Fri, Apr 19, 2013 at 11:46:22AM +0200, Michal Privoznik wrote: --- a/configure.ac +++ b/configure.ac @@ -10,7 +10,7 @@ AC_CANONICAL_HOST AM_SILENT_RULES([yes]) -LIBOSINFO_REQUIRED=0.2.3 +LIBOSINFO_REQUIRED=0.2.6 LIBVIRT_GCONFIG_REQUIRED=0.0.9 LIBVIRT_GOBJECT_REQUIRED=0.1.3 GOBJECT_INTROSPECTION_REQUIRED=0.10.8 The requirements in README should be updated with this change as well. And yes - they weren't updated previously :) ACK if you update the README. I've just sent a series making sure README and configure.ac always use the same minimum versions as this will scale better. I won't be changing the README in this patch if the other series gets ACKed. Christophe pgpyXj5zxicCa.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer 1/3] Fix 'dependancy' typo in README file
On Mon, Apr 22, 2013 at 06:57:10PM +0200, Christophe Fergeau wrote: --- README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README b/README index ed39b9d..b7ca356 100644 --- a/README +++ b/README @@ -24,7 +24,7 @@ Or to install into a private user specific location make make install -The following mandatory dependancies are required in order to +The following mandatory dependencies are required in order to build libvirt-designer libvirt-gconfig = 0.1.3 (part of libvirt-glib project releases) Speling eror fixes can be pushed under the trivial rule :-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer 3/3] Set min versions from configure in .spec file
On Mon, Apr 22, 2013 at 06:57:12PM +0200, Christophe Fergeau wrote: Minimum libvirt-gobject, libvirt-gconfig and libosinfo versions are set in configure.ac, it's better not to duplicate them in libvirt-designer.spec.in, it's too easy to get them out of sync. --- configure.ac | 2 +- libvirt-designer.spec.in | 8 2 files changed, 5 insertions(+), 5 deletions(-) ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/3] logging: Make log regexp more compact (and readable)
On 23/04/13 00:40, Jiri Denemark wrote: --- src/util/virlog.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/virlog.c b/src/util/virlog.c index 721c9bd..e31abb0 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -79,8 +79,8 @@ static int virLogEnd = 0; static regex_t *virLogRegex = NULL; -#define VIR_LOG_DATE_REGEX [0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9] -#define VIR_LOG_TIME_REGEX [0-9][0-9]:[0-9][0-9]:[0-9][0-9]\\.[0-9][0-9][0-9]\\+[0-9][0-9][0-9][0-9] +#define VIR_LOG_DATE_REGEX [0-9]{4}-[0-9]{2}-[0-9]{2} +#define VIR_LOG_TIME_REGEX [0-9]{2}:[0-9]{2}:[0-9]{2}\\.[0-9]{3}\\+[0-9]{4} #define VIR_LOG_PID_REGEX [0-9]+ #define VIR_LOG_LEVEL_REGEX debug|info|warning|error ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/3] qemu: Ignore libvirt logs when reading QEMU error output
On 23/04/13 00:40, Jiri Denemark wrote: When QEMU fails to start, libvirt read its error output and reports it back in an error message. However, when libvirtd is configured to log debug messages, one would get the following unhelpful garbage: virsh # start cd error: Failed to start domain cd error: internal error process exited while connecting to monitor: \ 2013-04-22 14:24:54.214+: 2194219: debug : virFileClose:72 : \ Closed fd 21 2013-04-22 14:24:54.214+: 2194219: debug : virFileClose:72 : \ Closed fd 27 2013-04-22 14:24:54.215+: 2194219: debug : virFileClose:72 : \ Closed fd 3 2013-04-22 14:24:54.215+: 2194220: debug : virExec:602 : Run \ hook 0x7feb8f600bf0 0x7feb86ef9300 2013-04-22 14:24:54.215+: 2194220: debug : qemuProcessHook:2507 \ : Obtaining domain lock 2013-04-22 14:24:54.216+: 2194220: debug : \ virDomainLockProcessStart:170 : plugin=0x7feb780261f0 \ dom=0x7feb7802a360 paused=1 fd=0x7feb86ef8ec4 2013-04-22 14:24:54.216+: 2194220: debug : \ virDomainLockManagerNew:128 : plugin=0x7feb780261f0 \ dom=0x7feb7802a360 withResources=1 2013-04-22 14:24:54.216+: 2194220: debug : \ virLockManagerPluginGetDriver:297 : plugin=0x7feb780261f0 2013-04-22 14:24:54.216+: 2194220: debug : \ virLockManagerNew:321 : driver=0x7feb8ef08640 type=0 nparams=5 \ params=0x7feb86ef8d60 flags=0 2013-04-22 14:24:54.216+000 instead of (the output with this patch applied): virsh # start cd error: Reconnected to the hypervisor error: Failed to start domain cd error: internal error process exited while connecting to monitor: \ char device redirected to /dev/pts/33 (label charserial0) qemu-system-x86_64: -drive file=/home/vm/systemrescuecd-x86-1.2.0.\ iso,if=none,id=drive-ide0-1-0,readonly=on,format=raw,cache=none: \ could not open disk image /home/vm/systemrescuecd-x86-1.2.0.iso: \ Permission denied --- src/qemu/qemu_process.c | 20 1 file changed, 4 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index a1fbd5f..4988d9b 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1665,21 +1665,6 @@ static void qemuProcessFreePtyPath(void *payload, const void *name ATTRIBUTE_UNU VIR_FREE(payload); } -static void -qemuProcessReadLogFD(int logfd, char *buf, int maxlen, int off) -{ -int ret; -char *tmpbuf = buf + off; - -ret = saferead(logfd, tmpbuf, maxlen - off - 1); -if (ret 0) { -ret = 0; -} - -tmpbuf[ret] = '\0'; -} - - static int qemuProcessWaitForMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -1734,6 +1719,7 @@ cleanup: virHashFree(paths); if (pos != -1 kill(vm-pid, 0) == -1 errno == ESRCH) { +int len; /* VM is dead, any other error raised in the interim is probably * not as important as the qemu cmdline output */ if (virQEMUCapsUsedQMP(qemuCaps)) { @@ -1745,7 +1731,9 @@ cleanup: goto closelog; } } -qemuProcessReadLogFD(logfd, buf, buf_size, strlen(buf)); + +len = strlen(buf); +qemuProcessReadLog(logfd, buf + len, buf_size - len - 1, 0); virReportError(VIR_ERR_INTERNAL_ERROR, _(process exited while connecting to monitor: %s), buf); ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] qemu: Move QEMU log reading into a separate function
On 23/04/13 00:40, Jiri Denemark wrote: --- src/qemu/qemu_process.c | 61 ++--- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index ce9f501..a1fbd5f 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -1346,6 +1346,41 @@ error: return ret; } +static int +qemuProcessReadLog(int fd, char *buf, int buflen, int off) +{ +char *filter_next = buf; +ssize_t bytes; +char *eol; + +buf[0] = '\0'; + +while (off buflen - 1) { +bytes = saferead(fd, buf + off, buflen - off - 1); +if (bytes 0) +return -1; +else if (bytes == 0) +break; + +off += bytes; +buf[off] = '\0'; + +/* Filter out debug messages from intermediate libvirt process */ +while ((eol = strchr(filter_next, '\n'))) { +*eol = '\0'; +if (virLogProbablyLogMessage(filter_next)) { +memmove(filter_next, eol + 1, off - (eol - buf)); +off -= eol + 1 - filter_next; +} else { +filter_next = eol + 1; +*eol = '\n'; +} +} +} + +return off; +} + typedef int qemuProcessLogHandleOutput(virDomainObjPtr vm, const char *output, int fd); @@ -1365,46 +1400,26 @@ qemuProcessReadLogOutput(virDomainObjPtr vm, int retries = (timeout*10); int got = 0; int ret = -1; -char *filter_next = buf; buf[0] = '\0'; while (retries) { -ssize_t func_ret, bytes; +ssize_t func_ret; int isdead = 0; -char *eol; func_ret = func(vm, buf, fd); if (kill(vm-pid, 0) == -1 errno == ESRCH) isdead = 1; -/* Any failures should be detected before we read the log, so we - * always have something useful to report on failure. */ -bytes = saferead(fd, buf+got, buflen-got-1); -if (bytes 0) { +got = qemuProcessReadLog(fd, buf, buflen, got); +if (got 0) { virReportSystemError(errno, _(Failure while reading %s log output), what); goto cleanup; } -got += bytes; -buf[got] = '\0'; - -/* Filter out debug messages from intermediate libvirt process */ -while ((eol = strchr(filter_next, '\n'))) { -*eol = '\0'; -VIR_ERROR(%s, filter_next); -if (virLogProbablyLogMessage(filter_next)) { -memmove(filter_next, eol + 1, got - (eol - buf)); -got -= eol + 1 - filter_next; -} else { -filter_next = eol + 1; -*eol = '\n'; -} -} - if (got == buflen-1) { virReportError(VIR_ERR_INTERNAL_ERROR, _(Out of space while reading %s log output: %s), ACK -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer 2/3] Automatically set min versions in README
On Mon, Apr 22, 2013 at 06:57:11PM +0200, Christophe Fergeau wrote: Minimum libvirt-gconfig and libosinfo versions appear in both README and configure.ac, which means they easily get out of sync. This commit renames README to README.in so that we can substitute the configure.ac version in the README file. This way they are always in sync. --- Makefile.am | 1 + README = README.in | 4 ++-- autogen.sh | 4 configure.ac| 5 + 4 files changed, 12 insertions(+), 2 deletions(-) rename README = README.in (93%) ACK, good idea. Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer PATCHv2 3/3] Rework disk bus type handling
On Fri, Apr 19, 2013 at 11:46:21AM +0200, Michal Privoznik wrote: On 18.04.2013 18:08, Christophe Fergeau wrote: +static OsinfoDevice * +gvir_designer_domain_get_fallback_disk_controller(GVirDesignerDomain *design, + GError **error) +{ +OsinfoEntity *dev = NULL; +OsinfoDeviceList *devices; +OsinfoFilter *filter; +int virt_type; + +filter = osinfo_filter_new(); +osinfo_filter_add_constraint(filter, OSINFO_DEVICE_PROP_CLASS, block); +devices = gvir_designer_domain_get_supported_devices(design, filter); +g_object_unref(G_OBJECT(filter)); + +if ((devices == NULL) || +(osinfo_list_get_length(OSINFO_LIST(devices)) == 0)) { No need for enclosing these two conditions in parentheses here ... +goto cleanup; +} + +virt_type = gvir_config_domain_get_virt_type(design-priv-config); +if ((virt_type == GVIR_CONFIG_DOMAIN_VIRT_QEMU) || +(virt_type == GVIR_CONFIG_DOMAIN_VIRT_KQEMU) || +(virt_type == GVIR_CONFIG_DOMAIN_VIRT_KVM)) { ... here ... +/* If using QEMU; we favour using virtio-block */ +OsinfoList *tmp_devices; +filter = osinfo_filter_new(); +osinfo_filter_add_constraint(filter, + OSINFO_ENTITY_PROP_ID, + GVIR_DESIGNER_VIRTIO_BLOCK_DEVICE_ID); +tmp_devices = osinfo_list_new_filtered(OSINFO_LIST(devices), filter); +if ((tmp_devices != NULL) +(osinfo_list_get_length(OSINFO_LIST(tmp_devices)) 0)) { ... and here. I much prefer having parentheses around conditions, this saves me some thinking effort with respect to operator priorities ;) I've removed the () in this patch, and in the sound/video patches from the other series I've sent. Christophe pgphoo0xk2O0r.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [libvirt-designer PATCHv2 9/9] Implement gvir_designer_domain_add_video()
On Thu, Apr 18, 2013 at 06:12:33PM +0200, Christophe Fergeau wrote: diff --git a/libvirt-designer/libvirt-designer-domain.c b/libvirt-designer/libvirt-designer-domain.c index 7aec002..73dbc8d 100644 --- a/libvirt-designer/libvirt-designer-domain.c +++ b/libvirt-designer/libvirt-designer-domain.c @@ -1153,7 +1153,6 @@ gvir_designer_domain_setup_guest(GVirDesignerDomain *design, gvir_designer_domain_add_console(design); gvir_designer_domain_add_input(design); -ret = TRUE; This is actually bogus and breaks make check, I don't know how it ended up being removed. I've dropped this hunk from my local copy. Christophe pgpnz3jzG2uFh.pgp Description: PGP signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] update input ip processing
On 04/21/2013 10:34 AM, Gene Czarcinski wrote: 1. Handle invalid ULong prefix specified. When parsing for @prefix as a ULong, a -2 can be returned if the specification is not a valid ULong. 2. Error out if address= is not specified. 3. Merge netmask process/tests under family tests. 4. Max sure that prefix does not exceed maximum. . Signed-off-by: Gene Czarcinski g...@czarc.net --- src/conf/network_conf.c | 118 +++- 1 file changed, 66 insertions(+), 52 deletions(-) diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 2b56ca7..1503ece 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -1136,7 +1136,8 @@ virNetworkIPDefParseXML(const char *networkName, xmlNodePtr cur, save; char *address = NULL, *netmask = NULL; -unsigned long prefix; +unsigned long prefix = 0; +int prefixRc; int result = -1; save = ctxt-node; @@ -1144,14 +1145,8 @@ virNetworkIPDefParseXML(const char *networkName, /* grab raw data from XML */ def-family = virXPathString(string(./@family), ctxt); -address = virXPathString(string(./@address), ctxt); -if (virXPathULong(string(./@prefix), ctxt, prefix) 0) -def-prefix = 0; -else -def-prefix = prefix; - -netmask = virXPathString(string(./@netmask), ctxt); +address = virXPathString(string(./@address), ctxt); if (address) { if (virSocketAddrParse(def-address, address, AF_UNSPEC) 0) { virReportError(VIR_ERR_XML_ERROR, Since I decided to just tweak a couple messages in this patch, I modified the existing log message here (not shown by diff) to: _(Invalid address '%s' in network '%s'), @@ -1160,23 +1155,66 @@ virNetworkIPDefParseXML(const char *networkName, goto cleanup; } +} else { +virReportError(VIR_ERR_XML_ERROR, + _(Network address must be specified in definition of network '%s'), _(Missing required address attribute in network '%s'), + networkName); +goto cleanup; +} + Structuring it this way leads to less indentation: if (!address) { log error goto cleanup; } if (virSocketAddrParse() 0) { log error goto cleanup; } +netmask = virXPathString(string(./@netmask), ctxt); +if (netmask) { +if (virSocketAddrParse(def-netmask, netmask, AF_UNSPEC) 0) { +virReportError(VIR_ERR_XML_ERROR, + _(Bad netmask '%s' in definition of network '%s'), _(Invalid netmask '%s' in network '%s'), + netmask, networkName); +goto cleanup; +} + +} Those two nested ifs can be combined into a single if. + +prefixRc = virXPathULong(string(./@prefix), ctxt, prefix); +if (prefixRc == -2) { +virReportError(VIR_ERR_XML_ERROR, + _(Invalid ULong value specified for prefix in definition of network '%s'), _(Invalid prefix in network '%s'), (I tweaked a few other messages, and will attach an interdiff of what I pushed at the bottom of this message) + networkName); +goto cleanup; } +if (prefixRc 0) +def-prefix = 0; +else +def-prefix = prefix; -/* validate family vs. address */ -if (def-family == NULL) { +/* validate address, etc. for each family */ +if ((def-family == NULL) || (STREQ(def-family, ipv4))) { if (!(VIR_SOCKET_ADDR_IS_FAMILY(def-address, AF_INET) || VIR_SOCKET_ADDR_IS_FAMILY(def-address, AF_UNSPEC))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(no family specified for non-IPv4 address '%s' in network '%s'), - address, networkName); + _(%s family specified for non-IPv4 address '%s' in network '%s'), + def-family == NULL? no : ipv4, address, networkName); goto cleanup; } -} else if (STREQ(def-family, ipv4)) { -if (!VIR_SOCKET_ADDR_IS_FAMILY(def-address, AF_INET)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _(family 'ipv4' specified for non-IPv4 address '%s' in network '%s'), - address, networkName); -goto cleanup; +if (netmask) { +if (!VIR_SOCKET_ADDR_IS_FAMILY(def-netmask, AF_INET)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _(network '%s' has invalid netmask '%s' for address '%s' (both must be IPv4)), + networkName, netmask, address); +goto cleanup; +} +if (def-prefix 0) { +
Re: [libvirt] [PATCH 0/3] Ignore libvirt logs when reading QEMU error output
On Mon, Apr 22, 2013 at 18:40:07 +0200, Jiri Denemark wrote: Jiri Denemark (3): qemu: Move QEMU log reading into a separate function qemu: Ignore libvirt logs when reading QEMU error output logging: Make log regexp more compact (and readable) src/qemu/qemu_process.c | 81 + src/util/virlog.c | 4 +-- 2 files changed, 44 insertions(+), 41 deletions(-) Pushed, thanks for the review. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3 1/5] qemu: call post-parse callbacks when parsing command line too
Assume format type is 'auto' when none is specified on qemu command line. --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 5 + src/libvirt_private.syms| 1 + src/qemu/qemu_command.c | 6 ++ tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml | 2 ++ tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-console-compat.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-empty.xml| 2 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml | 2 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-drive-boot-cdrom.xml | 2 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-drive-boot-disk.xml| 2 ++ .../qemuxml2argv-disk-drive-network-rbd-ceph-env.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-disk-floppy.xml | 3 +++ tests/qemuxml2argvdata/qemuxml2argv-disk-many.xml | 4 tests/qemuxml2argvdata/qemuxml2argv-disk-usb.xml| 2 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml | 4 tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml | 4 tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-address.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-machine-core-off.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-machine-core-on.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-migrate.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-no-reboot.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-user.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-net-virtio.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-nographics-vga.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-restore-v1.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-restore-v2.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-dev.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-file.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-many.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-pty.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-tcp.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-udp.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-unix.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-vc.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-smp.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-sound.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-watchdog.xml| 1 + 60 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dc0ecaa..8d57256 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2710,7 +2710,7 @@ virDomainDefPostParseDeviceIterator(virDomainDefPtr def ATTRIBUTE_UNUSED, } -static int +int virDomainDefPostParse(virDomainDefPtr def,
[libvirt] [PATCH v3 5/5] qemu: auto-add bridges and allow using them
Add a dry run address allocation to figure out how many bridges will be needed for all the devices without explicit addresses. Auto-add just enough bridges to put all the devices on, or up to the bridge with the largest specified index. --- src/conf/domain_conf.c | 26 ++- src/qemu/qemu_command.c| 211 + src/qemu/qemu_command.h| 4 +- tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml | 210 tests/qemuxml2xmltest.c| 1 + 5 files changed, 411 insertions(+), 41 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5740009..800c0a7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2578,6 +2578,9 @@ virDomainDefPostParseInternal(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED) { int i; +bool b; +int ret = -1; +virBitmapPtr bitmap = NULL; /* verify init path for container based domains */ if (STREQ(def-os.type, exe) !def-os.init) { @@ -2653,11 +2656,30 @@ virDomainDefPostParseInternal(virDomainDefPtr def, } } -return 0; +/* Verify that PCI controller indexes are unique */ +bitmap = virBitmapNew(def-ncontrollers); +for (i = 0; i def-ncontrollers; i++) { +if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { +ignore_value(virBitmapGetBit(bitmap, def-controllers[i]-idx, b)); +if (b) { +virReportError(VIR_ERR_XML_ERROR, + _(Multiple PCI controllers with index %d), + def-controllers[i]-idx); +goto cleanup; +} +ignore_value(virBitmapSetBit(bitmap, def-controllers[i]-idx)); +} +} +ret = 0; + +cleanup: +virBitmapFree(bitmap); + +return ret; no_memory: virReportOOMError(); -return -1; +goto cleanup; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3787ff1..ec7d0e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1197,6 +1197,8 @@ struct _qemuDomainPCIAddressSet { qemuDomainPCIAddressBus *used; virDevicePCIAddress lastaddr; size_t nbuses;/* allocation of 'used' */ +bool dryRun; /* on a dry run, new buses are auto-added + and addresses aren't saved in device infos */ }; @@ -1216,9 +1218,10 @@ static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN _(Only PCI domain 0 is available)); return false; } -if (addr-bus != 0) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(Only PCI bus 0 is available)); +if (addr-bus = addrs-nbuses) { +virReportError(VIR_ERR_XML_ERROR, + _(Only PCI buses up to %zu are available), + addrs-nbuses - 1); return false; } if (addr-function = QEMU_PCI_ADDRESS_FUNCTION_LAST) { @@ -1233,9 +1236,46 @@ static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN QEMU_PCI_ADDRESS_SLOT_LAST); return false; } +if (addr-slot == 0) { +if (addr-bus) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(Slot 0 is unusable on PCI bridges)); +return false; +} else { +virReportError(VIR_ERR_XML_ERROR, %s, + _(Slot 0 on bus 0 is reserved for the host bridge)); +return false; +} +} return true; } +/* Ensure addr fits in the address set, by expanding it if needed. + * Return value: + * -1 = OOM + * 0 = no action performed + * 0 = number of buses added + */ +static int +qemuDomainPCIAddressSetGrow(qemuDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) +{ +int add, i; + +add = addr-bus - addrs-nbuses + 1; +i = addrs-nbuses; +if (add = 0) +return 0; +if (VIR_EXPAND_N(addrs-used, addrs-nbuses, add) 0) { +virReportOOMError(); +return -1; +} +/* reserve slot 0 on the new buses */ +for (; i addrs-nbuses; i++) +addrs-used[i][0] = 0xFF; +return add; +} + static char *qemuPCIAddressAsString(virDevicePCIAddressPtr addr) { @@ -1273,6 +1313,9 @@ static int qemuCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return 0; } +if (addrs-dryRun qemuDomainPCIAddressSetGrow(addrs, addr) 0) +return -1; + if (!qemuPCIAddressValidate(addrs, addr)) return -1; @@ -1326,15 +1369,54 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, qemuDomainObjPrivatePtr priv = NULL; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { +int max_idx =
[libvirt] [PATCH v3 3/5] qemu: build command line for pci-bridge device
From: liguang lig.f...@cn.fujitsu.com --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 30 +- tests/qemuhelptest.c | 21 ++--- 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ef291c0..31e56fa 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -220,6 +220,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, machine-usb-opt, tpm-passthrough, tpm-tis, + + pci-bridge, /* 140 */ ); struct _virQEMUCaps { @@ -1347,6 +1349,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { virtio-rng-ccw, QEMU_CAPS_DEVICE_VIRTIO_RNG }, { rng-random, QEMU_CAPS_OBJECT_RNG_RANDOM }, { rng-egd, QEMU_CAPS_OBJECT_RNG_EGD }, +{ pci-bridge, QEMU_CAPS_DEVICE_PCI_BRIDGE }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4e76799..9c11157 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -178,6 +178,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_MACHINE_USB_OPT= 137, /* -machine xxx,usb=on/off */ QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 138, /* -tpmdev passthrough */ QEMU_CAPS_DEVICE_TPM_TIS = 139, /* -device tpm_tis */ +QEMU_CAPS_DEVICE_PCI_BRIDGE = 140, /* -device pci-bridge */ 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 37a961d..f052a91 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3606,6 +3606,24 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, break; +case VIR_DOMAIN_CONTROLLER_TYPE_PCI: +switch (def-model) { +case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: +if (def-idx == 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(PCI bridge index should be 0)); +goto error; +} +virBufferAsprintf(buf, pci-bridge,chassis_nr=%d,id=pci.%d, + def-idx, def-idx); +break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(wrong function called for pci-root)); +return NULL; +} +break; + /* We always get an IDE controller, whether we want it or not. */ case VIR_DOMAIN_CONTROLLER_TYPE_IDE: default: @@ -5791,7 +5809,11 @@ qemuBuildCommandLine(virConnectPtr conn, int contOrder[] = { /* We don't add an explicit IDE or FD controller because the * provided PIIX4 device already includes one. It isn't possible to - * remove the PIIX4. */ + * remove the PIIX4. + * + * We don't add PCI root controller either, because it's implicit, + * but we do add PCI bridges. */ +VIR_DOMAIN_CONTROLLER_TYPE_PCI, VIR_DOMAIN_CONTROLLER_TYPE_USB, VIR_DOMAIN_CONTROLLER_TYPE_SCSI, VIR_DOMAIN_CONTROLLER_TYPE_SATA, @@ -6405,6 +6427,12 @@ qemuBuildCommandLine(virConnectPtr conn, continue; } +/* Skip pci-root */ +if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI +cont-model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { +continue; +} + /* Only recent QEMU implements a SATA (AHCI) controller */ if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) { diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 7290183..eeba4d4 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -398,7 +398,8 @@ mymain(void) QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, QEMU_CAPS_DEVICE_USB_SERIAL, -QEMU_CAPS_DEVICE_USB_NET); +QEMU_CAPS_DEVICE_USB_NET, +QEMU_CAPS_DEVICE_PCI_BRIDGE); DO_TEST(qemu-kvm-0.12.3, 12003, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -507,7 +508,8 @@ mymain(void) QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, QEMU_CAPS_DEVICE_USB_SERIAL, -QEMU_CAPS_DEVICE_USB_NET); +QEMU_CAPS_DEVICE_USB_NET, +QEMU_CAPS_DEVICE_PCI_BRIDGE); DO_TEST(qemu-kvm-0.12.1.2-rhel61, 12001, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -572,7 +574,8 @@ mymain(void) QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, QEMU_CAPS_DEVICE_USB_SERIAL, -
[libvirt] [PATCH v3 0/5] qemu: add PCI bridge support
Add new 'pci' controller type with two models: pci-root - auto-added to machines with implicit pci bus pci-bridge - auto-added if the devices would not leave at least one slot empty on bus 0 or bus 0 is specified v3: moved the implicit PCI root addition to qemu's post parse callback, added an xml - xml test and schema validation rewrote implicit controller removal and search for free slots check for multiple pci controllers with the same index added documentation Ján Tomko (4): qemu: call post-parse callbacks when parsing command line too conf: add PCI controllers qemu: auto-add pci-root controller for pc machine types qemu: auto-add bridges and allow using them liguang (1): qemu: build command line for pci-bridge device docs/formatdomain.html.in | 22 +- docs/schemas/domaincommon.rng | 12 + src/conf/domain_conf.c | 51 +++- src/conf/domain_conf.h | 20 ++ src/libvirt_private.syms | 2 + src/qemu/qemu_capabilities.c | 3 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c| 296 + src/qemu/qemu_command.h| 5 +- src/qemu/qemu_domain.c | 67 - tests/domainsnapshotxml2xmlout/disk_snapshot.xml | 1 + tests/domainsnapshotxml2xmlout/external_vm.xml | 1 + tests/domainsnapshotxml2xmlout/full_domain.xml | 1 + tests/domainsnapshotxml2xmlout/metadata.xml| 1 + tests/qemuhelptest.c | 21 +- .../qemuxml2argvdata/qemuxml2argv-blkdeviotune.xml | 1 + .../qemuxml2argv-blkiotune-device.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-blkiotune.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml | 2 + .../qemuxml2argvdata/qemuxml2argv-boot-floppy.xml | 3 + .../qemuxml2argv-boot-menu-disable.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-boot-multi.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-boot-network.xml | 2 + tests/qemuxml2argvdata/qemuxml2argv-boot-order.xml | 1 + .../qemuxml2argv-channel-guestfwd.xml | 1 + .../qemuxml2argv-channel-virtio.xml| 1 + .../qemuxml2argv-clock-localtime.xml | 2 + tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml | 2 + .../qemuxml2argv-console-compat.xml| 2 + .../qemuxml2argv-console-virtio-many.xml | 1 + .../qemuxml2argv-cpu-eoi-disabled.xml | 1 + .../qemuxml2argv-cpu-eoi-enabled.xml | 1 + .../qemuxml2argv-cpu-host-kvmclock.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-cpu-kvmclock.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-cputune.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-disk-aio.xml | 1 + .../qemuxml2argv-disk-cdrom-empty.xml | 3 + tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml | 3 + .../qemuxml2argv-disk-drive-boot-cdrom.xml | 3 + .../qemuxml2argv-disk-drive-boot-disk.xml | 3 + .../qemuxml2argv-disk-drive-cache-directsync.xml | 1 + .../qemuxml2argv-disk-drive-cache-unsafe.xml | 1 + .../qemuxml2argv-disk-drive-cache-v1-none.xml | 1 + .../qemuxml2argv-disk-drive-cache-v1-wb.xml| 1 + .../qemuxml2argv-disk-drive-cache-v1-wt.xml| 1 + .../qemuxml2argv-disk-drive-cache-v2-none.xml | 1 + .../qemuxml2argv-disk-drive-cache-v2-wb.xml| 1 + .../qemuxml2argv-disk-drive-cache-v2-wt.xml| 1 + ...muxml2argv-disk-drive-error-policy-enospace.xml | 1 + .../qemuxml2argv-disk-drive-error-policy-stop.xml | 1 + ...rgv-disk-drive-error-policy-wreport-rignore.xml | 1 + .../qemuxml2argv-disk-drive-fat.xml| 1 + .../qemuxml2argv-disk-drive-fmt-qcow.xml | 1 + .../qemuxml2argv-disk-drive-network-gluster.xml| 1 + .../qemuxml2argv-disk-drive-network-iscsi-auth.xml | 1 + .../qemuxml2argv-disk-drive-network-iscsi.xml | 1 + .../qemuxml2argv-disk-drive-network-nbd-export.xml | 1 + ...xml2argv-disk-drive-network-nbd-ipv6-export.xml | 1 + .../qemuxml2argv-disk-drive-network-nbd-ipv6.xml | 1 + .../qemuxml2argv-disk-drive-network-nbd-unix.xml | 1 + .../qemuxml2argv-disk-drive-network-nbd.xml| 1 + ...emuxml2argv-disk-drive-network-rbd-ceph-env.xml | 2 + .../qemuxml2argv-disk-drive-network-rbd-ipv6.xml | 1 + .../qemuxml2argv-disk-drive-network-rbd.xml| 1 + .../qemuxml2argv-disk-drive-network-sheepdog.xml | 1 + .../qemuxml2argvdata/qemuxml2argv-disk-floppy.xml | 4 + tests/qemuxml2argvdata/qemuxml2argv-disk-many.xml | 5 + .../qemuxml2argvdata/qemuxml2argv-disk-mirror.xml | 1 + .../qemuxml2argv-disk-scsi-device.xml | 1 + .../qemuxml2argv-disk-scsi-disk-vpd.xml| 1 +
[libvirt] [PATCH v3 2/5] conf: add PCI controllers
Add new controller type 'pci' with models 'pci-root' and 'pci-bridge'. --- docs/formatdomain.html.in | 22 +- docs/schemas/domaincommon.rng | 12 src/conf/domain_conf.c| 21 - src/conf/domain_conf.h| 9 + 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 888c005..4a700f9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2124,7 +2124,7 @@ p Each controller has a mandatory attribute codetype/code, which must be one of ide, fdc, scsi, sata, usb, - ccid, or virtio-serial, and a mandatory + ccid, virtio-serial or pci, and a mandatory attribute codeindex/code which is the decimal integer describing in which order the bus controller is encountered (for use in codecontroller/code attributes @@ -2177,6 +2177,26 @@ lt;/devicesgt; .../pre +p + PCI controllers have an optional codemodel/code attribute with + possible values codepci-root/code or codepci-bridge/code. + For machine types which provide an implicit pci bus, the pci-root + controller with index=0 is auto-added and required to use PCI devices. + PCI root has no address. + PCI bridges are auto-added if there are too many devices to fit on + the one bus provided by pci-root, or a PCI bus number greater than zero + was specified. (span class=sincesince 1.0.5/span) +/p +pre + ... + lt;devicesgt; +lt;controller type='pci' index='0' model='pci-root'/gt; +lt;controller type='pci' index='1' model='pci-bridge'gt; + lt;address type='pci' domain='0' bus='0' slot='5' function='0' multifunction=off'/gt; +lt;/controllergt; + lt;/devicesgt; + .../pre + h4a name=elementsLeaseDevice leases/a/h4 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..cf91c2d 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1475,6 +1475,18 @@ ref name=usbmaster/ /optional /group + !-- pci has an optional attribute model -- + group +attribute name=type + valuepci/value +/attribute +attribute name=model + choice +valuepci-root/value +valuepci-bridge/value + /choice +/attribute + /group !-- virtio-serial has optional ports and vectors -- group attribute name=type diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8d57256..1e7de52 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -300,7 +300,12 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, sata, virtio-serial, ccid, - usb) + usb, + pci) + +VIR_ENUM_IMPL(virDomainControllerModelPCI, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST, + pci-root, + pci-bridge) VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, auto, @@ -5144,6 +5149,8 @@ virDomainControllerModelTypeFromString(const virDomainControllerDefPtr def, return virDomainControllerModelSCSITypeFromString(model); else if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_USB) return virDomainControllerModelUSBTypeFromString(model); +else if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) +return virDomainControllerModelPCITypeFromString(model); return -1; } @@ -5261,6 +5268,16 @@ virDomainControllerDefParseXML(xmlNodePtr node, } break; } +case VIR_DOMAIN_CONTROLLER_TYPE_PCI: +switch (def-model) { +case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: +if (def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(pci-root controller should not + have an address)); +goto error; +} +} default: break; @@ -13488,6 +13505,8 @@ virDomainControllerModelTypeToString(virDomainControllerDefPtr def, return virDomainControllerModelSCSITypeToString(model); else if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_USB) return virDomainControllerModelUSBTypeToString(model); +else if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) +return virDomainControllerModelPCITypeToString(model); return NULL; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 89515de..3cb626b 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -696,11 +696,19 @@ enum virDomainControllerType { VIR_DOMAIN_CONTROLLER_TYPE_VIRTIO_SERIAL, VIR_DOMAIN_CONTROLLER_TYPE_CCID, VIR_DOMAIN_CONTROLLER_TYPE_USB, +
[libvirt] [PATCH qom-cpu 7/9] target-i386: Use FeatureWord loop on filter_features_for_kvm()
Instead of open-coding the filtering code for each feature word, change the existing code to use the feature_word_info array, that have exactly the same CPUID eax/ecx/register values for each feature word. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- target-i386/cpu.c | 24 +++- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 757749c..bdb94a7 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1638,24 +1638,14 @@ static void filter_features_for_kvm(X86CPU *cpu) { CPUX86State *env = cpu-env; KVMState *s = kvm_state; +FeatureWord w; -env-features[FEAT_1_EDX] = -kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX); -env-features[FEAT_1_ECX] = -kvm_arch_get_supported_cpuid(s, 1, 0, R_ECX); -env-features[FEAT_8000_0001_EDX] = -kvm_arch_get_supported_cpuid(s, 0x8001, 0, R_EDX); -env-features[FEAT_8000_0001_ECX] = -kvm_arch_get_supported_cpuid(s, 0x8001, 0, R_ECX); -env-features[FEAT_SVM] = -kvm_arch_get_supported_cpuid(s, 0x800A, 0, R_EDX); -env-features[FEAT_7_0_EBX] = -kvm_arch_get_supported_cpuid(s, 7, 0, R_EBX); -env-features[FEAT_KVM] = -kvm_arch_get_supported_cpuid(s, KVM_CPUID_FEATURES, 0, R_EAX); -env-features[FEAT_C000_0001_EDX] = -kvm_arch_get_supported_cpuid(s, 0xC001, 0, R_EDX); - +for (w = 0; w FEATURE_WORDS; w++) { +FeatureWordInfo *wi = feature_word_info[w]; +env-features[w] = kvm_arch_get_supported_cpuid(s, wi-cpuid_eax, +wi-cpuid_ecx, +wi-cpuid_reg); +} } #endif -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 0/9] x86: feature words array (v11) + feature-words property
This series includes the previous replace cpuid_*features fields with a feature word array series. The first 4 patches already have a Reviewed-by from Igor, they correspond to v10 plus a small indent fix requested by him. As the cpuid_*features series was holding my feature-words/filtered-features series (previously sent as RFC), I am now sending both as a single series, to try to get feature-words/filtered-features some attention and try to get it included in 1.5. The feature-words/filtered-features mechanism is very important for libvirt, to allow it to ensure the guest is getting the required set of CPU features, as configured by the user. Eduardo Habkost (9): target-i386: cleanup: Group together level, xlevel, xlevel2 fields target-i386/kvm.c: Code formatting changes target-i386/cpu.c: Break lines so they don't get too long target-i386: Replace cpuid_*features fields with a feature word array target-i386: Add ECX information to FeatureWordInfo target-i386: Add feature-words property target-i386: Use FeatureWord loop on filter_features_for_kvm() target-i386: Introduce X86CPU.filtered_features field target-i386: Add filtered-features property to X86CPU .gitignore| 2 + Makefile.objs | 7 +- bsd-user/elfload.c| 2 +- bsd-user/main.c | 4 +- hw/i386/kvm/clock.c | 2 +- linux-user/elfload.c | 2 +- linux-user/main.c | 4 +- qapi-schema.json | 31 +++ target-i386/cpu-qom.h | 3 + target-i386/cpu.c | 501 +- target-i386/cpu.h | 15 +- target-i386/helper.c | 4 +- target-i386/kvm.c | 5 +- target-i386/misc_helper.c | 14 +- target-i386/translate.c | 10 +- 15 files changed, 385 insertions(+), 221 deletions(-) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 6/9] target-i386: Add feature-words property
This property will be useful for libvirt, as libvirt already has logic based on low-level feature bits (not feature names), so it will be really easy to convert the current libvirt logic to something using the feature-words property. The property will have two main use cases: - Checking host capabilities, by checking the features of the host CPU model - Checking which features are enabled on each CPU model Example output: $ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1] --property=feature-words item[0].cpuid-register: EDX item[0].cpuid-input-eax: 2147483658 item[0].features: 0 item[1].cpuid-register: EAX item[1].cpuid-input-eax: 1073741825 item[1].features: 0 item[2].cpuid-register: EDX item[2].cpuid-input-eax: 3221225473 item[2].features: 0 item[3].cpuid-register: ECX item[3].cpuid-input-eax: 2147483649 item[3].features: 101 item[4].cpuid-register: EDX item[4].cpuid-input-eax: 2147483649 item[4].features: 563346425 item[5].cpuid-register: EBX item[5].cpuid-input-eax: 7 item[5].features: 0 item[5].cpuid-input-ecx: 0 item[6].cpuid-register: ECX item[6].cpuid-input-eax: 1 item[6].features: 2155880449 item[7].cpuid-register: EDX item[7].cpuid-input-eax: 1 item[7].features: 126614521 Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- Changes v1 - v2: * Merge the non-qapi and qapi patches, to keep series simpler * Use the feature word array series as base, so we don't have to set the feature word values one-by-one in the code * Change type name of property from x86-cpu-feature-words to X86CPUFeatureWordInfo * Remove cpu-qapi-schema.json and simply add the type definitions to qapi-schema.json, to keep the changes simpler * This required compiling qapi-types.o and qapi-visit.o into *-user as well --- .gitignore| 2 ++ Makefile.objs | 7 +- qapi-schema.json | 31 target-i386/cpu.c | 70 +-- 4 files changed, 97 insertions(+), 13 deletions(-) diff --git a/.gitignore b/.gitignore index 487813a..71408e3 100644 --- a/.gitignore +++ b/.gitignore @@ -21,6 +21,8 @@ linux-headers/asm qapi-generated qapi-types.[ch] qapi-visit.[ch] +cpu-qapi-types.[ch] +cpu-qapi-visit.[ch] qmp-commands.h qmp-marshal.c qemu-doc.html diff --git a/Makefile.objs b/Makefile.objs index a473348..1835d37 100644 --- a/Makefile.objs +++ b/Makefile.objs @@ -78,10 +78,15 @@ common-obj-$(CONFIG_SMARTCARD_NSS) += $(libcacard-y) ## # qapi -common-obj-y += qmp-marshal.o qapi-visit.o qapi-types.o +common-obj-y += qmp-marshal.o common-obj-y += qmp.o hmp.o endif +## +# some qapi visitors are used by both system and user emulation: + +common-obj-y += qapi-visit.o qapi-types.o + ### # Target-independent parts used in system and user emulation common-obj-y += qemu-log.o diff --git a/qapi-schema.json b/qapi-schema.json index 751d3c2..424434f 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3505,3 +3505,34 @@ '*asl_compiler_rev': 'uint32', '*file': 'str', '*data': 'str' }} + +# @X86CPURegister32 +# +# A X86 32-bit register +# +# Since: 1.5 +## +{ 'enum': 'X86CPURegister32', + 'data': [ 'EAX', 'EBX', 'ECX', 'EDX', 'ESP', 'EBP', 'ESI', 'EDI' ] } + +## +# @X86CPUFeatureWordInfo +# +# Information about a X86 CPU feature word +# +# @cpuid-input-eax: Input EAX value for CPUID instruction for that feature word +# +# @cpuid-input-ecx: #optional Input ECX value for CPUID instruction for that +# feature word +# +# @cpuid-register: Output register containing the feature bits +# +# @features: value of output register, containing the feature bits +# +# Since: 1.5 +## +{ 'type': 'X86CPUFeatureWordInfo', + 'data': { 'cpuid-input-eax': 'int', +'*cpuid-input-ecx': 'int', +'cpuid-register': 'X86CPURegister32', +'features': 'int' } } diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 314931e..757749c 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -30,6 +30,8 @@ #include qemu/config-file.h #include qapi/qmp/qerror.h +#include qapi-types.h +#include qapi-visit.h #include qapi/visitor.h #include sysemu/arch_init.h @@ -194,23 +196,34 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, }; +typedef struct X86RegisterInfo32 { +/* Name of register */ +const char *name; +/* QAPI enum value register */ +X86CPURegister32 qapi_enum; +} X86RegisterInfo32; + +#define REGISTER(reg) \ +[R_##reg] = { .name = #reg, .qapi_enum = X86_C_P_U_REGISTER32_##reg } +X86RegisterInfo32 x86_reg_info_32[CPU_NB_REGS32] = { +REGISTER(EAX), +REGISTER(ECX), +REGISTER(EDX), +REGISTER(EBX), +REGISTER(ESP), +
[libvirt] [PATCH qom-cpu 9/9] target-i386: Add filtered-features property to X86CPU
This property will contain all the features that were removed from the CPU because they are not supported by the host. This way, libvirt or other management tools can emulate the check/enforce behavior by checking if filtered-properties is all zeroes, before starting the guest. Example output where some features were missing: $ ./install/bin/qemu-system-x86_64 -enable-kvm -cpu Haswell,check -S -qmp unix:/tmp/m,server,nowait warning: host doesn't support requested feature: CPUID.01H:ECX.fma [bit 12] warning: host doesn't support requested feature: CPUID.01H:ECX.movbe [bit 22] warning: host doesn't support requested feature: CPUID.01H:ECX.tsc-deadline [bit 24] warning: host doesn't support requested feature: CPUID.01H:ECX.xsave [bit 26] warning: host doesn't support requested feature: CPUID.01H:ECX.avx [bit 28] warning: host doesn't support requested feature: CPUID.07H:EBX.fsgsbase [bit 0] warning: host doesn't support requested feature: CPUID.07H:EBX.bmi1 [bit 3] warning: host doesn't support requested feature: CPUID.07H:EBX.hle [bit 4] warning: host doesn't support requested feature: CPUID.07H:EBX.avx2 [bit 5] warning: host doesn't support requested feature: CPUID.07H:EBX.smep [bit 7] warning: host doesn't support requested feature: CPUID.07H:EBX.bmi2 [bit 8] warning: host doesn't support requested feature: CPUID.07H:EBX.erms [bit 9] warning: host doesn't support requested feature: CPUID.07H:EBX.invpcid [bit 10] warning: host doesn't support requested feature: CPUID.07H:EBX.rtm [bit 11] [...] $ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1] --property=filtered-features item[0].cpuid-register: EDX item[0].cpuid-input-eax: 2147483658 item[0].features: 0 item[1].cpuid-register: EAX item[1].cpuid-input-eax: 1073741825 item[1].features: 0 item[2].cpuid-register: EDX item[2].cpuid-input-eax: 3221225473 item[2].features: 0 item[3].cpuid-register: ECX item[3].cpuid-input-eax: 2147483649 item[3].features: 0 item[4].cpuid-register: EDX item[4].cpuid-input-eax: 2147483649 item[4].features: 0 item[5].cpuid-register: EBX item[5].cpuid-input-eax: 7 item[5].features: 4025 item[5].cpuid-input-ecx: 0 item[6].cpuid-register: ECX item[6].cpuid-input-eax: 1 item[6].features: 356519936 item[7].cpuid-register: EDX item[7].cpuid-input-eax: 1 item[7].features: 0 Example output when no feature is missing: $ ./install/bin/qemu-system-x86_64 -enable-kvm -cpu Nehalem,enforce -S -qmp unix:/tmp/m,server,nowait [...] $ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1] --property=filtered-features item[0].cpuid-register: EDX item[0].cpuid-input-eax: 2147483658 item[0].features: 0 item[1].cpuid-register: EAX item[1].cpuid-input-eax: 1073741825 item[1].features: 0 item[2].cpuid-register: EDX item[2].cpuid-input-eax: 3221225473 item[2].features: 0 item[3].cpuid-register: ECX item[3].cpuid-input-eax: 2147483649 item[3].features: 0 item[4].cpuid-register: EDX item[4].cpuid-input-eax: 2147483649 item[4].features: 0 item[5].cpuid-register: EBX item[5].cpuid-input-eax: 7 item[5].features: 0 item[5].cpuid-input-ecx: 0 item[6].cpuid-register: ECX item[6].cpuid-input-eax: 1 item[6].features: 0 item[7].cpuid-register: EDX item[7].cpuid-input-eax: 1 item[7].features: 0 Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- target-i386/cpu.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 1178c5f..5bcb79c 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1373,11 +1373,11 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque, cpu-env.tsc_khz = value / 1000; } +/* Generic getter for feature-words and filtered-features properties */ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque, const char *name, Error **errp) { -X86CPU *cpu = X86_CPU(obj); -CPUX86State *env = cpu-env; +uint32_t *array = (uint32_t *)opaque; FeatureWord w; Error *err = NULL; X86CPUFeatureWordInfo word_infos[FEATURE_WORDS] = { }; @@ -1391,7 +1391,7 @@ static void x86_cpu_get_feature_words(Object *obj, Visitor *v, void *opaque, qwi-has_cpuid_input_ecx = wi-cpuid_needs_ecx; qwi-cpuid_input_ecx = wi-cpuid_ecx; qwi-cpuid_register = x86_reg_info_32[wi-cpuid_reg].qapi_enum; -qwi-features = env-features[w]; +qwi-features = array[w]; /* List will be in reverse order, but order shouldn't matter */ list_entries[w].next = list; @@ -2386,7 +2386,10 @@ static void x86_cpu_initfn(Object *obj) x86_cpuid_set_tsc_freq, NULL, NULL, NULL); object_property_add(obj, feature-words, X86CPUFeatureWordInfo, x86_cpu_get_feature_words, -NULL, NULL, NULL, NULL); +
[libvirt] [PATCH qom-cpu 5/9] target-i386: Add ECX information to FeatureWordInfo
FEAT_7_0_EBX uses ECX as input, so we have to take that into account when reporting feature word values. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- target-i386/cpu.c | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 110ef98..314931e 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -151,8 +151,10 @@ static const char *cpuid_7_0_ebx_feature_name[] = { typedef struct FeatureWordInfo { const char **feat_names; -uint32_t cpuid_eax; /* Input EAX for CPUID */ -int cpuid_reg; /* R_* register constant */ +uint32_t cpuid_eax; /* Input EAX for CPUID */ +bool cpuid_needs_ecx; /* CPUID instruction uses ECX as input */ +uint32_t cpuid_ecx; /* Input ECX value for CPUID */ +int cpuid_reg;/* output register (R_* constant) */ } FeatureWordInfo; static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { @@ -186,7 +188,9 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = { }, [FEAT_7_0_EBX] = { .feat_names = cpuid_7_0_ebx_feature_name, -.cpuid_eax = 7, .cpuid_reg = R_EBX, +.cpuid_eax = 7, +.cpuid_needs_ecx = true, .cpuid_ecx = 0, +.cpuid_reg = R_EBX, }, }; -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 1/9] target-i386: cleanup: Group together level, xlevel, xlevel2 fields
Consolidate level, xlevel, xlevel2 fields in x86_def_t and CPUX86State. Signed-off-by: Eduardo Habkost ehabk...@redhat.com Reviewed-By: Igor Mammedov imamm...@redhat.com --- Changes v9: * Merged target-i386: Move cpuid_xlevel, cpuid_xlevel2 fields in X86CPU and target-i386: Move xlevel/xlevel2 in struct x86_def_t in a single patch --- target-i386/cpu.c | 4 ++-- target-i386/cpu.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index e2302d8..732cafd 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -349,6 +349,8 @@ static void add_flagname_to_bitmaps(const char *flagname, typedef struct x86_def_t { const char *name; uint32_t level; +uint32_t xlevel; +uint32_t xlevel2; /* vendor is zero-terminated, 12 character ASCII string */ char vendor[CPUID_VENDOR_SZ + 1]; int family; @@ -356,11 +358,9 @@ typedef struct x86_def_t { int stepping; uint32_t features, ext_features, ext2_features, ext3_features; uint32_t kvm_features, svm_features; -uint32_t xlevel; char model_id[48]; /* Store the results of Centaur's CPUID instructions */ uint32_t ext4_features; -uint32_t xlevel2; /* The feature bits on CPUID[EAX=7,ECX=0].EBX */ uint32_t cpuid_7_0_ebx_features; } x86_def_t; diff --git a/target-i386/cpu.h b/target-i386/cpu.h index a1614e8..c621359 100644 --- a/target-i386/cpu.h +++ b/target-i386/cpu.h @@ -836,19 +836,19 @@ typedef struct CPUX86State { /* processor features (e.g. for CPUID insn) */ uint32_t cpuid_level; +uint32_t cpuid_xlevel; +uint32_t cpuid_xlevel2; uint32_t cpuid_vendor1; uint32_t cpuid_vendor2; uint32_t cpuid_vendor3; uint32_t cpuid_version; uint32_t cpuid_features; uint32_t cpuid_ext_features; -uint32_t cpuid_xlevel; uint32_t cpuid_model[12]; uint32_t cpuid_ext2_features; uint32_t cpuid_ext3_features; uint32_t cpuid_apic_id; /* Store the results of Centaur's CPUID instructions */ -uint32_t cpuid_xlevel2; uint32_t cpuid_ext4_features; /* Flags from CPUID[EAX=7,ECX=0].EBX */ uint32_t cpuid_7_0_ebx_features; -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 2/9] target-i386/kvm.c: Code formatting changes
Add appropriate spaces around operators, and break line where it needs to be broken to allow feature-words array to be introduced without having too-long lines. Signed-off-by: Eduardo Habkost ehabk...@redhat.com Reviewed-By: Igor Mammedov imamm...@redhat.com --- Changes v9: * 1-char alignment change: keep the opening parenthesis of both sides of the == operator starting at the same column --- target-i386/kvm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/target-i386/kvm.c b/target-i386/kvm.c index 0e7cc81..b5bff33 100644 --- a/target-i386/kvm.c +++ b/target-i386/kvm.c @@ -613,7 +613,8 @@ int kvm_arch_init_vcpu(CPUState *cs) cpuid_data.cpuid.nent = cpuid_i; if (((env-cpuid_version 8)0xF) = 6 - (env-cpuid_features(CPUID_MCE|CPUID_MCA)) == (CPUID_MCE|CPUID_MCA) + (env-cpuid_features (CPUID_MCE|CPUID_MCA)) == + (CPUID_MCE|CPUID_MCA) kvm_check_extension(cs-kvm_state, KVM_CAP_MCE) 0) { uint64_t mcg_cap; int banks; -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH qom-cpu 3/9] target-i386/cpu.c: Break lines so they don't get too long
Break lines on kvm_check_features_against_host(), kvm_cpu_fill_host(), and builtin_x86_defs, so they don't get too long once the *_features fields are replaced by an array. Signed-off-by: Eduardo Habkost ehabk...@redhat.com Reviewed-By: Igor Mammedov imamm...@redhat.com --- Changes v9: * Merged all Break lines patches from previous series into a single patch --- target-i386/cpu.c | 270 -- 1 file changed, 180 insertions(+), 90 deletions(-) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index 732cafd..73ae2ef 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -423,13 +423,17 @@ static x86_def_t builtin_x86_defs[] = { .family = 6, .model = 2, .stepping = 3, -.features = PPRO_FEATURES | +.features = +PPRO_FEATURES | CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36, -.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT, -.ext2_features = (PPRO_FEATURES CPUID_EXT2_AMD_ALIASES) | +.ext_features = +CPUID_EXT_SSE3 | CPUID_EXT_CX16 | CPUID_EXT_POPCNT, +.ext2_features = +(PPRO_FEATURES CPUID_EXT2_AMD_ALIASES) | CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, -.ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | +.ext3_features = +CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A, .xlevel = 0x800A, }, @@ -440,12 +444,15 @@ static x86_def_t builtin_x86_defs[] = { .family = 16, .model = 2, .stepping = 3, -.features = PPRO_FEATURES | +.features = +PPRO_FEATURES | CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36 | CPUID_VME | CPUID_HT, -.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 | +.ext_features = +CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_CX16 | CPUID_EXT_POPCNT, -.ext2_features = (PPRO_FEATURES CPUID_EXT2_AMD_ALIASES) | +.ext2_features = +(PPRO_FEATURES CPUID_EXT2_AMD_ALIASES) | CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX | CPUID_EXT2_3DNOW | CPUID_EXT2_3DNOWEXT | CPUID_EXT2_MMXEXT | CPUID_EXT2_FFXSR | CPUID_EXT2_PDPE1GB | CPUID_EXT2_RDTSCP, @@ -453,9 +460,11 @@ static x86_def_t builtin_x86_defs[] = { CPUID_EXT3_CR8LEG, CPUID_EXT3_MISALIGNSSE, CPUID_EXT3_3DNOWPREFETCH, CPUID_EXT3_OSVW, CPUID_EXT3_IBS */ -.ext3_features = CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | +.ext3_features = +CPUID_EXT3_LAHF_LM | CPUID_EXT3_SVM | CPUID_EXT3_ABM | CPUID_EXT3_SSE4A, -.svm_features = CPUID_SVM_NPT | CPUID_SVM_LBRV, +.svm_features = +CPUID_SVM_NPT | CPUID_SVM_LBRV, .xlevel = 0x801A, .model_id = AMD Phenom(tm) 9550 Quad-Core Processor }, @@ -466,15 +475,19 @@ static x86_def_t builtin_x86_defs[] = { .family = 6, .model = 15, .stepping = 11, -.features = PPRO_FEATURES | +.features = +PPRO_FEATURES | CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36 | CPUID_VME | CPUID_DTS | CPUID_ACPI | CPUID_SS | CPUID_HT | CPUID_TM | CPUID_PBE, -.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 | +.ext_features = +CPUID_EXT_SSE3 | CPUID_EXT_MONITOR | CPUID_EXT_SSSE3 | CPUID_EXT_DTES64 | CPUID_EXT_DSCPL | CPUID_EXT_VMX | CPUID_EXT_EST | CPUID_EXT_TM2 | CPUID_EXT_CX16 | CPUID_EXT_XTPR | CPUID_EXT_PDCM, -.ext2_features = CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, -.ext3_features = CPUID_EXT3_LAHF_LM, +.ext2_features = +CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, +.ext3_features = +CPUID_EXT3_LAHF_LM, .xlevel = 0x8008, .model_id = Intel(R) Core(TM)2 Duo CPU T7700 @ 2.40GHz, }, @@ -486,19 +499,23 @@ static x86_def_t builtin_x86_defs[] = { .model = 6, .stepping = 1, /* Missing: CPUID_VME, CPUID_HT */ -.features = PPRO_FEATURES | +.features = +PPRO_FEATURES | CPUID_MTRR | CPUID_CLFLUSH | CPUID_MCA | CPUID_PSE36, /* Missing: CPUID_EXT_POPCNT, CPUID_EXT_MONITOR */ -.ext_features = CPUID_EXT_SSE3 | CPUID_EXT_CX16, +.ext_features = +CPUID_EXT_SSE3 | CPUID_EXT_CX16, /* Missing: CPUID_EXT2_PDPE1GB, CPUID_EXT2_RDTSCP */ -.ext2_features = (PPRO_FEATURES CPUID_EXT2_AMD_ALIASES) | +.ext2_features = +(PPRO_FEATURES CPUID_EXT2_AMD_ALIASES) | CPUID_EXT2_LM | CPUID_EXT2_SYSCALL | CPUID_EXT2_NX, /* Missing: CPUID_EXT3_LAHF_LM,
[libvirt] [PATCH qom-cpu 8/9] target-i386: Introduce X86CPU.filtered_features field
This field will contain the feature bits that were filtered out because of missing host support. Signed-off-by: Eduardo Habkost ehabk...@redhat.com --- target-i386/cpu-qom.h | 3 +++ target-i386/cpu.c | 9 ++--- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h index 08f9eb6..159378f 100644 --- a/target-i386/cpu-qom.h +++ b/target-i386/cpu-qom.h @@ -65,6 +65,9 @@ typedef struct X86CPU { /* public */ CPUX86State env; + +/* Features that were filtered out because of missing host capabilities */ +uint32_t filtered_features[FEATURE_WORDS]; } X86CPU; static inline X86CPU *x86_env_get_cpu(CPUX86State *env) diff --git a/target-i386/cpu.c b/target-i386/cpu.c index bdb94a7..1178c5f 100644 --- a/target-i386/cpu.c +++ b/target-i386/cpu.c @@ -1642,9 +1642,12 @@ static void filter_features_for_kvm(X86CPU *cpu) for (w = 0; w FEATURE_WORDS; w++) { FeatureWordInfo *wi = feature_word_info[w]; -env-features[w] = kvm_arch_get_supported_cpuid(s, wi-cpuid_eax, -wi-cpuid_ecx, -wi-cpuid_reg); +uint32_t host_feat = kvm_arch_get_supported_cpuid(s, wi-cpuid_eax, + wi-cpuid_ecx, + wi-cpuid_reg); +uint32_t requested_features = env-features[w]; +env-features[w] = host_feat; +cpu-filtered_features[w] = requested_features ~env-features[w]; } } #endif -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [libvirt-designer] Add missing files to EXTRA_DIST
This fixes make distcheck --- I've pushed this under the trivial rule. Christophe Makefile.am | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Makefile.am b/Makefile.am index 9ef319a..726d475 100644 --- a/Makefile.am +++ b/Makefile.am @@ -10,6 +10,8 @@ EXTRA_DIST = \ $(PACKAGE).spec \ $(pkgconfig_DATA:%.pc=%.pc.in) \ AUTHORS.in \ + GNUmakefile \ + maint.mk \ README.in \ $(NULL) -- 1.8.1.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 1/5] qemu: call post-parse callbacks when parsing command line too
On 04/22/2013 02:43 PM, Ján Tomko wrote: Assume format type is 'auto' when none is specified on qemu command line. --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 5 + src/libvirt_private.syms| 1 + src/qemu/qemu_command.c | 6 ++ tests/qemuxml2argvdata/qemuxml2argv-boot-cdrom.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-boot-floppy.xml | 2 ++ tests/qemuxml2argvdata/qemuxml2argv-boot-network.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-clock-localtime.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-clock-utc.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-console-compat.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom-empty.xml| 2 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-cdrom.xml | 2 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-drive-boot-cdrom.xml | 2 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-drive-boot-disk.xml| 2 ++ .../qemuxml2argv-disk-drive-network-rbd-ceph-env.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-disk-floppy.xml | 3 +++ tests/qemuxml2argvdata/qemuxml2argv-disk-many.xml | 4 tests/qemuxml2argvdata/qemuxml2argv-disk-usb.xml| 2 ++ tests/qemuxml2argvdata/qemuxml2argv-disk-virtio.xml | 4 tests/qemuxml2argvdata/qemuxml2argv-disk-xenvbd.xml | 4 tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl-fullscreen.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-sdl.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-sasl.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-socket.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc-tls.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-graphics-vnc.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-hostdev-pci-address.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-hostdev-usb-address.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-input-usbmouse.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-input-usbtablet.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-machine-core-off.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-machine-core-on.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-migrate.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-acpi.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-disable-s3.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-disable-suspends.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-enable-s4.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-no-reboot.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-misc-uuid.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-eth-ifname.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-eth.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-net-user.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-net-virtio.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-nographics-vga.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp.xml| 1 + tests/qemuxml2argvdata/qemuxml2argv-qemu-ns-no-env.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-restore-v1.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-restore-v2.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-dev.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-file.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-many.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-pty.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-tcp.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-udp.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-unix.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-serial-vc.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-smp.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-sound.xml | 1 + tests/qemuxml2argvdata/qemuxml2argv-watchdog.xml| 1 + 60 files changed, 86 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index dc0ecaa..8d57256 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2710,7 +2710,7 @@ virDomainDefPostParseDeviceIterator(virDomainDefPtr def
Re: [libvirt] [PATCH v3 2/5] conf: add PCI controllers
On 04/22/2013 02:43 PM, Ján Tomko wrote: Add new controller type 'pci' with models 'pci-root' and 'pci-bridge'. --- docs/formatdomain.html.in | 22 +- docs/schemas/domaincommon.rng | 12 src/conf/domain_conf.c| 21 - src/conf/domain_conf.h| 9 + 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 888c005..4a700f9 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2124,7 +2124,7 @@ p Each controller has a mandatory attribute codetype/code, which must be one of ide, fdc, scsi, sata, usb, - ccid, or virtio-serial, and a mandatory + ccid, virtio-serial or pci, and a mandatory attribute codeindex/code which is the decimal integer describing in which order the bus controller is encountered (for use in codecontroller/code attributes @@ -2177,6 +2177,26 @@ lt;/devicesgt; .../pre +p + PCI controllers have an optional codemodel/code attribute with + possible values codepci-root/code or codepci-bridge/code. + For machine types which provide an implicit pci bus, the pci-root + controller with index=0 is auto-added and required to use PCI devices. + PCI root has no address. + PCI bridges are auto-added if there are too many devices to fit on + the one bus provided by pci-root, or a PCI bus number greater than zero + was specified. (span class=sincesince 1.0.5/span) Just so that it's clear that it's not automatic-only, you should also say something like a pci-bridge device can be manually added in the domain's configuration, but care should be taken to not have any gaps in the sequence of index attributes when there are multiple pci controllers. +/p +pre + ... + lt;devicesgt; +lt;controller type='pci' index='0' model='pci-root'/gt; +lt;controller type='pci' index='1' model='pci-bridge'gt; + lt;address type='pci' domain='0' bus='0' slot='5' function='0' multifunction=off'/gt; +lt;/controllergt; + lt;/devicesgt; + .../pre + h4a name=elementsLeaseDevice leases/a/h4 p diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 3976b82..cf91c2d 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1475,6 +1475,18 @@ ref name=usbmaster/ /optional /group + !-- pci has an optional attribute model -- + group +attribute name=type + valuepci/value +/attribute +attribute name=model + choice +valuepci-root/value +valuepci-bridge/value + /choice +/attribute + /group !-- virtio-serial has optional ports and vectors -- group attribute name=type diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 8d57256..1e7de52 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -300,7 +300,12 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, sata, virtio-serial, ccid, - usb) + usb, + pci) + +VIR_ENUM_IMPL(virDomainControllerModelPCI, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST, + pci-root, + pci-bridge) VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, auto, @@ -5144,6 +5149,8 @@ virDomainControllerModelTypeFromString(const virDomainControllerDefPtr def, return virDomainControllerModelSCSITypeFromString(model); else if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_USB) return virDomainControllerModelUSBTypeFromString(model); +else if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) +return virDomainControllerModelPCITypeFromString(model); return -1; } @@ -5261,6 +5268,16 @@ virDomainControllerDefParseXML(xmlNodePtr node, } break; } +case VIR_DOMAIN_CONTROLLER_TYPE_PCI: +switch (def-model) { +case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: +if (def-info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(pci-root controller should not + have an address)); +goto error; +} +} default: break; @@ -13488,6 +13505,8 @@ virDomainControllerModelTypeToString(virDomainControllerDefPtr def, return virDomainControllerModelSCSITypeToString(model); else if (def-type == VIR_DOMAIN_CONTROLLER_TYPE_USB) return virDomainControllerModelUSBTypeToString(model); +else if (def-type ==
Re: [libvirt] [PATCH v3 3/5] qemu: build command line for pci-bridge device
On 04/22/2013 02:43 PM, Ján Tomko wrote: From: liguang lig.f...@cn.fujitsu.com Add a Signed-off-by: line for yourself. --- src/qemu/qemu_capabilities.c | 3 +++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 30 +- tests/qemuhelptest.c | 21 ++--- 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index ef291c0..31e56fa 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -220,6 +220,8 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, machine-usb-opt, tpm-passthrough, tpm-tis, + + pci-bridge, /* 140 */ ); struct _virQEMUCaps { @@ -1347,6 +1349,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { virtio-rng-ccw, QEMU_CAPS_DEVICE_VIRTIO_RNG }, { rng-random, QEMU_CAPS_OBJECT_RNG_RANDOM }, { rng-egd, QEMU_CAPS_OBJECT_RNG_EGD }, +{ pci-bridge, QEMU_CAPS_DEVICE_PCI_BRIDGE }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 4e76799..9c11157 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -178,6 +178,7 @@ enum virQEMUCapsFlags { QEMU_CAPS_MACHINE_USB_OPT= 137, /* -machine xxx,usb=on/off */ QEMU_CAPS_DEVICE_TPM_PASSTHROUGH = 138, /* -tpmdev passthrough */ QEMU_CAPS_DEVICE_TPM_TIS = 139, /* -device tpm_tis */ +QEMU_CAPS_DEVICE_PCI_BRIDGE = 140, /* -device pci-bridge */ 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 37a961d..f052a91 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -3606,6 +3606,24 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, break; +case VIR_DOMAIN_CONTROLLER_TYPE_PCI: +switch (def-model) { +case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: +if (def-idx == 0) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(PCI bridge index should be 0)); +goto error; +} +virBufferAsprintf(buf, pci-bridge,chassis_nr=%d,id=pci.%d, + def-idx, def-idx); +break; +case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: +virReportError(VIR_ERR_INTERNAL_ERROR, %s, + _(wrong function called for pci-root)); +return NULL; +} +break; + /* We always get an IDE controller, whether we want it or not. */ case VIR_DOMAIN_CONTROLLER_TYPE_IDE: default: @@ -5791,7 +5809,11 @@ qemuBuildCommandLine(virConnectPtr conn, int contOrder[] = { /* We don't add an explicit IDE or FD controller because the * provided PIIX4 device already includes one. It isn't possible to - * remove the PIIX4. */ + * remove the PIIX4. + * + * We don't add PCI root controller either, because it's implicit, + * but we do add PCI bridges. */ because it's implicit in the machinetype. Move the closing */ down to the next line. +VIR_DOMAIN_CONTROLLER_TYPE_PCI, VIR_DOMAIN_CONTROLLER_TYPE_USB, VIR_DOMAIN_CONTROLLER_TYPE_SCSI, VIR_DOMAIN_CONTROLLER_TYPE_SATA, @@ -6405,6 +6427,12 @@ qemuBuildCommandLine(virConnectPtr conn, continue; } +/* Skip pci-root */ +if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI +cont-model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { +continue; +} + /* Only recent QEMU implements a SATA (AHCI) controller */ if (cont-type == VIR_DOMAIN_CONTROLLER_TYPE_SATA) { if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_ICH9_AHCI)) { diff --git a/tests/qemuhelptest.c b/tests/qemuhelptest.c index 7290183..eeba4d4 100644 --- a/tests/qemuhelptest.c +++ b/tests/qemuhelptest.c @@ -398,7 +398,8 @@ mymain(void) QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, QEMU_CAPS_DEVICE_USB_SERIAL, -QEMU_CAPS_DEVICE_USB_NET); +QEMU_CAPS_DEVICE_USB_NET, +QEMU_CAPS_DEVICE_PCI_BRIDGE); DO_TEST(qemu-kvm-0.12.3, 12003, 1, 0, QEMU_CAPS_VNC_COLON, QEMU_CAPS_NO_REBOOT, @@ -507,7 +508,8 @@ mymain(void) QEMU_CAPS_DEVICE_CIRRUS_VGA, QEMU_CAPS_DEVICE_VMWARE_SVGA, QEMU_CAPS_DEVICE_USB_SERIAL, -QEMU_CAPS_DEVICE_USB_NET); +QEMU_CAPS_DEVICE_USB_NET, +QEMU_CAPS_DEVICE_PCI_BRIDGE);
Re: [libvirt] [PATCH qom-cpu 6/9] target-i386: Add feature-words property
On 04/22/2013 01:00 PM, Eduardo Habkost wrote: This property will be useful for libvirt, as libvirt already has logic based on low-level feature bits (not feature names), so it will be really easy to convert the current libvirt logic to something using the feature-words property. The property will have two main use cases: - Checking host capabilities, by checking the features of the host CPU model - Checking which features are enabled on each CPU model Example output: $ ./QMP/qmp --path=/tmp/m qom-get --path=/machine/unattached/device[1] --property=feature-words If I'm not mistaken, the QMP counterpart that libvirt will use is: { execute:qom-get, arguments: { path:/machine/unattached/device[1], property:feature-words } } item[0].cpuid-register: EDX item[0].cpuid-input-eax: 2147483658 item[0].features: 0 item[1].cpuid-register: EAX item[1].cpuid-input-eax: 1073741825 item[1].features: 0 item[2].cpuid-register: EDX item[2].cpuid-input-eax: 3221225473 item[2].features: 0 item[3].cpuid-register: ECX item[3].cpuid-input-eax: 2147483649 item[3].features: 101 item[4].cpuid-register: EDX item[4].cpuid-input-eax: 2147483649 item[4].features: 563346425 item[5].cpuid-register: EBX item[5].cpuid-input-eax: 7 item[5].features: 0 item[5].cpuid-input-ecx: 0 item[6].cpuid-register: ECX item[6].cpuid-input-eax: 1 item[6].features: 2155880449 item[7].cpuid-register: EDX item[7].cpuid-input-eax: 1 item[7].features: 126614521 And this would then be returned as a JSON array containing struct members looking like this: +{ 'type': 'X86CPUFeatureWordInfo', + 'data': { 'cpuid-input-eax': 'int', +'*cpuid-input-ecx': 'int', +'cpuid-register': 'X86CPURegister32', +'features': 'int' } } Looks reasonable (and better than what we've had in the past!), although I'll let Jiri Denemark give final say on whether it meets libvirt's needs. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 4/5] qemu: auto-add pci-root controller for pc machine types
On 04/22/2013 02:43 PM, Ján Tomko wrote: controller type='pci' index='0' model='pci-root'/ is auto-added to pc* machine types. Without this controller PCI bus 0 is not available and no PCI addresses are assigned by default. Since older libvirt supported PCI bus 0 even without this controller, it is removed from the XML when migrating. --- src/conf/domain_conf.c | 2 +- src/conf/domain_conf.h | 6 ++ src/libvirt_private.syms | 1 + src/qemu/qemu_command.c| 57 -- src/qemu/qemu_command.h| 3 +- src/qemu/qemu_domain.c | 67 +- [ + tons of test xml files] 162 files changed, 269 insertions(+), 23 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1e7de52..5740009 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -9762,7 +9762,7 @@ virDomainLookupVcpuPin(virDomainDefPtr def, return NULL; } -static int +int virDomainDefMaybeAddController(virDomainDefPtr def, int type, int idx, diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3cb626b..565f0f8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2553,6 +2553,12 @@ int virDomainObjListExport(virDomainObjListPtr doms, virDomainVcpuPinDefPtr virDomainLookupVcpuPin(virDomainDefPtr def, int vcpuid); +int +virDomainDefMaybeAddController(virDomainDefPtr def, + int type, + int idx, + int model); + char *virDomainDefGetDefaultEmulator(virDomainDefPtr def, virCapsPtr caps); #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 32b4ae8..ca324de 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -116,6 +116,7 @@ virDomainDefFree; virDomainDefGenSecurityLabelDef; virDomainDefGetDefaultEmulator; virDomainDefGetSecurityLabelDef; +virDomainDefMaybeAddController; virDomainDefParseFile; virDomainDefParseNode; virDomainDefParseString; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index f052a91..3787ff1 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1196,6 +1196,7 @@ typedef uint8_t qemuDomainPCIAddressBus[QEMU_PCI_ADDRESS_SLOT_LAST]; struct _qemuDomainPCIAddressSet { qemuDomainPCIAddressBus *used; virDevicePCIAddress lastaddr; +size_t nbuses;/* allocation of 'used' */ }; @@ -1206,6 +1207,10 @@ struct _qemuDomainPCIAddressSet { static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UNUSED, virDevicePCIAddressPtr addr) { +if (addrs-nbuses == 0) { +virReportError(VIR_ERR_XML_ERROR, %s, _(No PCI buses available)); +return false; +} if (addr-domain != 0) { virReportError(VIR_ERR_XML_ERROR, %s, _(Only PCI domain 0 is available)); @@ -1321,7 +1326,15 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, qemuDomainObjPrivatePtr priv = NULL; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { -if (!(addrs = qemuDomainPCIAddressSetCreate(def))) +int nbuses = 0; +int i; + +for (i = 0; i def-ncontrollers; i++) { +if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) +nbuses++; +} + +if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses))) goto cleanup; if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) 0) @@ -1366,16 +1379,25 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, return qemuDomainAssignPCIAddresses(def, qemuCaps, obj); } -qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def) +qemuDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, + unsigned int nbuses) { qemuDomainPCIAddressSetPtr addrs; +int i; if (VIR_ALLOC(addrs) 0) goto no_memory; -if (VIR_ALLOC_N(addrs-used, 1) 0) +if (VIR_ALLOC_N(addrs-used, nbuses) 0) goto no_memory; +addrs-nbuses = nbuses; + +/* reserve slot 0 in every bus - it's used by the host bridge on bus 0 + * and unusable on PCI bridges */ +for (i = 0; i nbuses; i++) +addrs-used[i][0] = 0xFF; + if (virDomainDeviceInfoIterate(def, qemuCollectPCIAddress, addrs) 0) goto error; @@ -1604,12 +1626,6 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, virDevicePCIAddressPtr addrptr; unsigned int *func = tmp_addr.function; - -/* Reserve slot 0 for
Re: [libvirt] [PATCH v3 5/5] qemu: auto-add bridges and allow using them
(before anything else - you committed an unresolved merge conflict in qemu_command.h. You'll need to remove the extra blah text.) Hopefully Eric can once again review the logic of the code that determines what bridges need to be auto-added, and assign PCI addresses to devices, since he was kind enough to review it last time :-) On 04/22/2013 02:43 PM, Ján Tomko wrote: Add a dry run address allocation to figure out how many bridges will be needed for all the devices without explicit addresses. Auto-add just enough bridges to put all the devices on, or up to the bridge with the largest specified index. Ah, so then we don't need to warn about gaps in the index sequence. --- src/conf/domain_conf.c | 26 ++- src/qemu/qemu_command.c| 211 + src/qemu/qemu_command.h| 4 +- tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml | 210 tests/qemuxml2xmltest.c| 1 + 5 files changed, 411 insertions(+), 41 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-bridge.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 5740009..800c0a7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2578,6 +2578,9 @@ virDomainDefPostParseInternal(virDomainDefPtr def, virCapsPtr caps ATTRIBUTE_UNUSED) { int i; +bool b; +int ret = -1; +virBitmapPtr bitmap = NULL; /* verify init path for container based domains */ if (STREQ(def-os.type, exe) !def-os.init) { @@ -2653,11 +2656,30 @@ virDomainDefPostParseInternal(virDomainDefPtr def, } } -return 0; +/* Verify that PCI controller indexes are unique */ +bitmap = virBitmapNew(def-ncontrollers); +for (i = 0; i def-ncontrollers; i++) { +if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { +ignore_value(virBitmapGetBit(bitmap, def-controllers[i]-idx, b)); +if (b) { +virReportError(VIR_ERR_XML_ERROR, + _(Multiple PCI controllers with index %d), + def-controllers[i]-idx); +goto cleanup; +} +ignore_value(virBitmapSetBit(bitmap, def-controllers[i]-idx)); +} +} +ret = 0; + +cleanup: +virBitmapFree(bitmap); + +return ret; I don't see where we do something like this for the other controller types. We should (separate patch though, of course :-) no_memory: virReportOOMError(); -return -1; +goto cleanup; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 3787ff1..ec7d0e6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1197,6 +1197,8 @@ struct _qemuDomainPCIAddressSet { qemuDomainPCIAddressBus *used; virDevicePCIAddress lastaddr; size_t nbuses;/* allocation of 'used' */ +bool dryRun; /* on a dry run, new buses are auto-added + and addresses aren't saved in device infos */ }; @@ -1216,9 +1218,10 @@ static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN _(Only PCI domain 0 is available)); return false; } -if (addr-bus != 0) { -virReportError(VIR_ERR_XML_ERROR, %s, - _(Only PCI bus 0 is available)); +if (addr-bus = addrs-nbuses) { +virReportError(VIR_ERR_XML_ERROR, + _(Only PCI buses up to %zu are available), + addrs-nbuses - 1); return false; } if (addr-function = QEMU_PCI_ADDRESS_FUNCTION_LAST) { @@ -1233,9 +1236,46 @@ static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN QEMU_PCI_ADDRESS_SLOT_LAST); return false; } +if (addr-slot == 0) { +if (addr-bus) { +virReportError(VIR_ERR_XML_ERROR, %s, + _(Slot 0 is unusable on PCI bridges)); +return false; +} else { +virReportError(VIR_ERR_XML_ERROR, %s, + _(Slot 0 on bus 0 is reserved for the host bridge)); +return false; +} You can move the duplicated return false; out of the if and else, to a single occurrence. +} return true; } +/* Ensure addr fits in the address set, by expanding it if needed. + * Return value: + * -1 = OOM + * 0 = no action performed + * 0 = number of buses added + */ +static int +qemuDomainPCIAddressSetGrow(qemuDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) +{ +int add, i; + +add = addr-bus - addrs-nbuses + 1; +i = addrs-nbuses; +if (add = 0) +return 0; +
Re: [libvirt] [PATCHv2] Configure native vlan modes on Open vSwitch ports
(I'm not sure what you did differently when you sent this mail, but somehow your mailer botched the In-Reply-To: header, which broke the threaded display in Thunderbird. No big deal, but I thought you might want to know.) On 04/22/2013 12:51 PM, james robson wrote: Date: Thu, 18 Apr 2013 14:14:32 -0400 From: Laine Stump la...@laine.org To: libvir-list@redhat.com Subject: Re: [libvirt] [PATCHv2] Configure native vlan modes on Open vSwitch ports Message-ID: 51703808.2000...@laine.org Content-Type: text/plain; charset=ISO-8859-1 On 04/18/2013 01:44 PM, james robson wrote: Hello, Has any one been able to review this yet? I realise that the 'Since 1.0.3' in the doc page is now out of date, but is the code itself acceptable? I was hoping that someone with more knowledge of Open vSwitch and/or vlan tagging/trunking/native mode would repond to the message (Kyle?) but there was silence instead... diff --git a/tests/networkxml2xmlin/openvswitch-net.xml b/tests/networkxml2xmlin/openvswitch-net.xml index a3d82b1..93c49d5 100644 --- a/tests/networkxml2xmlin/openvswitch-net.xml +++ b/tests/networkxml2xmlin/openvswitch-net.xml @@ -21,4 +21,13 @@ parameters profileid='alice-profile'/ /virtualport /portgroup + portgroup name='tagged' +vlan native_mode='tagged' native_tag='123' + tag id='555'/ + tag id='444'/ +/vlan +virtualport + parameters profileid='tagged-profile'/ +/virtualport + /portgroup As brought up again in a separate conversation today, we prefer to use camelCase rather than underscored in attribute and element names. So, if we were to use the layout you're proposing, the attributes should be called nativeMode and nativeTag. However, I'm wondering if there might be a better way to structure it. What about this? vlan trunk='yes' tag id='123' native='tagged|untagged'/ (or whatever values are appropriate) Sounds like this can be native='yes', since there is only the possiblity of a tag being the native tag, or *not* being the native tag. tag id='555'/ tag id='444'/ /vlan Do I understand correctly that native mode is telling what to do with packets that come in untagged, and that (using your nomenclature native_mode='yes' native_tag='123' means when an untagged packet come in from this interface, it should be tagged as 123 before forwarding? That is correct, setting the native vlan changes how an untagged packet is handled when it enters the port. The difference between the 'tagged' and 'untagged' modes is in how packets on the native vlan are processed before exiting the port. And what happens when native_mode='yes' but there is no native_tag? In that case you configuration is invalid, and will get an error. Okay, then doing the config the way I suggest would eliminate that possibility, so it has an upside :-) (that's what I was trying to describe with tag id='123' native='untagged'/, but I don't even know if that makes sense, because I don't know exactly what is the native vlan tag and what is done with it :-) That arrangement would make sense, I chose the arrangement I did for two main reasons. There can only be one native vlan on a port, making it an attribute of the 'vlan' tag enforces this. Also, I wanted to keep the validation and processing separate rather than add 'if native' branches to the loops that operate on the vlan id list. I can see the advantage of having a single setting to configure the native vlan, rather than the two attributes I proposed. If the new suggestion is preferred I can rework my patch to use that format. I think it's simpler, to do it that way, yes. Also, is it valid to have a native_mode/native_tag if trunk='no'? (right now trunk is automatically set to 'yes' if there is more than one vlan tag) It isn't valid to have trunk='no' and the native settings. Therefore vlan trunk='no' native_mode='tagged' native_tag='123' will get an error if you try to enter it. If no trunk attribute is set explicitly then it will be set to 'yes'. This means vlan native_mode='tagged' native_tag='123' is equivalent to vlan trunk='yes' native_mode='tagged' native_tag='123'. Likewise, if you have more than one id tag='x'/ element, trunk will automatically be set to yes. So in the end, if you don't foresee any problems with it, I think I do prefer this: vlan trunk='yes' tag id='123' native='yes|no'/ (default is 'no', only one allowed to be yes) tag id='555'/ tag id='444'/ /vlan Note that we'll be going into freeze for 1.0.5 soon, so if you are able to rework the patch within the next couple days, it should go into libvirt-1.0.5 (which *might* end up in Fedora 19?) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH v3 5/5] qemu: auto-add bridges and allow using them
On 04/22/2013 12:43 PM, Ján Tomko wrote: Add a dry run address allocation to figure out how many bridges will be needed for all the devices without explicit addresses. Auto-add just enough bridges to put all the devices on, or up to the bridge with the largest specified index. --- In addition to Laine's comments, +virBitmapPtr bitmap = NULL; /* verify init path for container based domains */ if (STREQ(def-os.type, exe) !def-os.init) { @@ -2653,11 +2656,30 @@ virDomainDefPostParseInternal(virDomainDefPtr def, } } -return 0; +/* Verify that PCI controller indexes are unique */ +bitmap = virBitmapNew(def-ncontrollers); This limits the bitmap to the number of controllers that I passed in, but your commit message makes it sound like I can pass in a controller for index 1 and index 2 while letting the code auto-insert the controller for index 0. If that's true, then... +for (i = 0; i def-ncontrollers; i++) { +if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { +ignore_value(virBitmapGetBit(bitmap, def-controllers[i]-idx, b)); ...attempting to get bit 2 (based on the explicit 2 in def-controllers[i]-idx) will fail as out-of-range,... +if (b) { +virReportError(VIR_ERR_XML_ERROR, + _(Multiple PCI controllers with index %d), + def-controllers[i]-idx); +goto cleanup; +} +ignore_value(virBitmapSetBit(bitmap, def-controllers[i]-idx)); +} +} ...and the attempt to set will also fail. Which means that a similar example of passing in two controllers that both try to use index 2, and let 0 and 1 be auto-populated, won't detect the collision. Do we know what the maximum index will be? Is it time to add a growable bitmap? Should we separate this duplicate detection code into a separate patch? +/* Ensure addr fits in the address set, by expanding it if needed. + * Return value: + * -1 = OOM + * 0 = no action performed + * 0 = number of buses added + */ +static int +qemuDomainPCIAddressSetGrow(qemuDomainPCIAddressSetPtr addrs, + virDevicePCIAddressPtr addr) Indentation is off. @@ -1326,15 +1369,54 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, qemuDomainObjPrivatePtr priv = NULL; if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { +int max_idx = 0; int nbuses = 0; int i; +int rv; for (i = 0; i def-ncontrollers; i++) { -if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) +if (def-controllers[i]-type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { +if (def-controllers[i]-idx max_idx) +max_idx = def-controllers[i]-idx; nbuses++; +} +} This looks like a more accurate determination of the max bus number; should you move the duplicate detection here instead? + +if (nbuses 0 max_idx = nbuses) +nbuses = max_idx + 1; You change nbuses, but only if it is already 1, but then... + +if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) { +virDomainDeviceInfo info; +/* 1st pass to figure out how many PCI bridges we need */ +if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, true))) +goto cleanup; +if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) 0) +goto cleanup; +/* Reserve 1 extra slot for a (potential) bridge */ +if (qemuDomainPCIAddressSetNextAddr(addrs, info) 0) +goto cleanup; + +for (i = 1; i addrs-nbuses; i++) { +if ((rv = virDomainDefMaybeAddController( +def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, +i, VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE)) 0) +goto cleanup; +/* If we added a new bridge, we will need one more address */ +if (rv 0 qemuDomainPCIAddressSetNextAddr(addrs, info) 0) +goto cleanup; +} +nbuses = addrs-nbuses; +qemuDomainPCIAddressSetFree(addrs); +addrs = NULL; + +} else if (nbuses 1) { ...read nbuses if the capability is not present. Would it be any simpler to just change this to 'else if (max_idx 0)'? +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s, + _(PCI bridges are not supported + by this QEMU binary)); +goto cleanup; } -if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses))) +if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, false))) goto cleanup; if (qemuAssignDevicePCISlots(def, qemuCaps,