[libvirt] cpulimit and kvm process
Manao ahoana, Hello, Bonjour, I would like to launch several KVM guests on a multicore CPU. The number of the KVM process is over the number of physical cores. I would like to limit each KVM process to say... 10% of CPU I first use cpulimit Would you know some better way to limit them? it's in order to avoid 4 VM to hog all the 4 hardware cores. I also use all the livbirt tools, if there si any sol Misaotra, Thanks, Merci. -- Architecte Informatique chez Blueline/Gulfsat: Administration Systeme, Recherche Developpement +261 34 56 000 19 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Ejecting ISO from cdrom
Hello, how can I remove ISO image from cdrom in running domain, please? I tried procedure which is described on your wiki ( http://wiki.libvirt.org/page/QEMUSwitchToLibvirt#eject_DEV) but it doesn't work for me: divinus:/kvm/iso # virsh attach-disk --type cdrom --mode readonly leon.i-tux.cz hdc error: command 'attach-disk' requires target option divinus:/kvm/iso # virsh attach-disk --type cdrom --mode readonly leon.i-tux.cz --target hdc error: command 'attach-disk' requires source option divinus:/kvm/iso # virsh attach-disk --type cdrom --mode readonly leon.i-tux.cz --source --target hdc error: expected syntax: --source string Thank you, Jaromír Červenka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] phyp: Verify that domain XML contains at least one disk element
2010/9/30 Eric Blake ebl...@redhat.com:
On 09/30/2010 01:18 PM, Matthias Bolte wrote:
phypBuildLpar expects that at least one disk element is provided.
---
src/phyp/phyp_driver.c | 18 +++---
1 files changed, 11 insertions(+), 7 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index ab12392..0a7d6f9 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -3715,13 +3715,17 @@ phypBuildLpar(virConnectPtr conn, virDomainDefPtr
def)
goto err;
}
- if (def-ndisks 0) {
- if (!def-disks[0]-src) {
- PHYP_ERROR(VIR_ERR_XML_ERROR,%s,
- _(Field \src\ under \disk\ on the domain XML
file is
- missing.));
- goto err;
- }
+ if (def-ndisks 1) {
+ PHYP_ERROR(VIR_ERR_XML_ERROR, %s,
+ _(Domain XML must contain at least one \disk\
element.));
+ goto err;
+ }
ACK.
Thanks, pushed.
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Ejecting ISO from cdrom
On 10/01/2010 09:37 AM, Zdenek Styblik wrote: On 10/01/2010 09:17 AM, Jaromír Červenka wrote: Hello, how can I remove ISO image from cdrom in running domain, please? I tried procedure which is described on your wiki ( http://wiki.libvirt.org/page/QEMUSwitchToLibvirt#eject_DEV) but it doesn't work for me: divinus:/kvm/iso # virsh attach-disk --type cdrom --mode readonly leon.i-tux.cz hdc error: command 'attach-disk' requires target option divinus:/kvm/iso # virsh attach-disk --type cdrom --mode readonly leon.i-tux.cz --target hdc error: command 'attach-disk' requires source option divinus:/kvm/iso # virsh attach-disk --type cdrom --mode readonly leon.i-tux.cz --source --target hdc error: expected syntax: --source string Thank you, Jaromír Červenka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list Jaromir, virsh # attach-disk --type cdrom --source /home/ftp/linux/slackware64-13.1/slackware64-13.1-install-dvd.iso --mode readonly nagios --target hdc Disk attached successfully Please note, that you've provided three (3) commands and each is missing something; each of them is missing this and that (or I got it wrong and you were trying it). Please, re-check commands you've posted. I also think source can't be empty; at least it makes no sense to me. :) I haven't even checked what I'm typing; just copied what you've posted, yet committed full command to virsh, not parts. I hope it helps, Zdenek Ok, my bad. I should read more carefully and so on. I confirm this looks like a bug. I too couldn't figure out how to detach disk resp. unmount ISO (with or without wiki help). I've tried to get more info out of virt-manager, however I've given up after moment, because it seems they're shifting XML rather than using visrh commands. Have a nice day, Zdenek -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: sty...@turnovfree.net jabber: sty...@jabber.turnovfree.net -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 12/12] food for thought
On Wed, Sep 29, 2010 at 06:09:22PM -0600, Eric Blake wrote: Here's where I ran out of time for the day. I'm much less familiar with xen than with qemu, so I have no idea how to tell if xen's documented domain/vcpu_avail (which is what we want for current vcpus) is usable in contrast to domain/vcpus (the maximum amount). For that matter, I'm not even sure if modifying the Sxpr parsing/generating code is enough to make xen use the new attribute, or what else might be involved. Hints on what I need to do from here are greatly appreciated. I would think augmenting the sexpr should be sufficient for this but the problem is really to find out when the feature is available, and I don't know how to do this reliably either (except trying and if there is an identifiable error keep it disabled). Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ 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 01/12] vcpu: improve cpuset attribute
On Wed, Sep 29, 2010 at 06:02:05PM -0600, Eric Blake wrote: The vcpu cpuset=... attribute has been available since commit e193b5dd, but without documentation or RNG validation. * docs/schemas/domain.rng (vcpu): Further validate cpuset. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.c: Fix typos. ACK, actually independant fom the other patches, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ 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 03/12] vcpu: add new public API
On Wed, Sep 29, 2010 at 06:02:07PM -0600, Eric Blake wrote:
API agreed on in
https://www.redhat.com/archives/libvir-list/2010-September/msg00456.html
* include/libvirt/libvirt.h.in (virDomainVcpuFlags)
(virDomainSetVcpusFlags, virDomainGetVcpusFlags): New
declarations.
* src/libvirt_public.syms: Export new symbols.
---
However, in implementing things, I'm wondering if I should use the names:
VIR_DOMAIN_VCPU_CONFIG (instead of VIR_DOMAIN_VCPU_PERSISTENT)
VIR_DOMAIN_VCPU_LIVE (instead of VIR_DOMAIN_VCPU_ACTIVE)
to match virDomainDeviceModifyFlags, where _CONFIG and _LIVE have
the same semantics of setting one or both aspects of a domain.
include/libvirt/libvirt.h.in | 14 ++
src/libvirt_public.syms |6 ++
2 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
index b45f7ec..6bac8f1 100644
--- a/include/libvirt/libvirt.h.in
+++ b/include/libvirt/libvirt.h.in
@@ -826,8 +826,22 @@ struct _virVcpuInfo {
};
typedef virVcpuInfo *virVcpuInfoPtr;
+typedef enum {
+/* Must choose at least one of these two bits; SetVcpus can choose both
*/
+VIR_DOMAIN_VCPU_ACTIVE = (1 0), /* Affect active domain */
+VIR_DOMAIN_VCPU_PERSISTENT = (1 1), /* Affect next boot */
+
+/* Additional flags to be bit-wise OR'd in */
+VIR_DOMAIN_VCPU_MAXIMUM= (1 2), /* Max rather than current count
*/
+} virDomainVcpuFlags;
+
int virDomainSetVcpus (virDomainPtr domain,
unsigned int nvcpus);
+int virDomainSetVcpusFlags (virDomainPtr domain,
+ unsigned int nvcpus,
+ unsigned int flags);
+int virDomainGetVcpusFlags (virDomainPtr domain,
+ unsigned int flags);
int virDomainPinVcpu(virDomainPtr domain,
unsigned int vcpu,
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 849c163..4e89723 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -405,4 +405,10 @@ LIBVIRT_0.8.2 {
virDomainCreateWithFlags;
} LIBVIRT_0.8.1;
+LIBVIRT_0.8.5 {
+global:
+virDomainGetVcpusFlags;
+virDomainSetVcpusFlags;
+} LIBVIRT_0.8.2;
+
# define new API here using predicted next version number
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
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 04/12] vcpu: define internal driver API
On Wed, Sep 29, 2010 at 06:02:08PM -0600, Eric Blake wrote: * src/driver.h (virDrvDomainSetVcpusFlags) (virDrvDomainGetVcpusFlags): New typedefs. (_virDriver): New callback members. * src/esx/esx_driver.c (esxDriver): Add stub for driver. * src/lxc/lxc_driver.c (lxcDriver): Likewise. * src/opennebula/one_driver.c (oneDriver): Likewise. * src/openvz/openvz_driver.c (openvzDriver): Likewise. * src/phyp/phyp_driver.c (phypDriver): Likewise. * src/qemu/qemu_driver.c (qemuDriver): Likewise. * src/remote/remote_driver.c (remote_driver): Likewise. * src/test/test_driver.c (testDriver): Likewise. * src/uml/uml_driver.c (umlDriver): Likewise. * src/vbox/vbox_tmpl.c (Driver): Likewise. * src/xen/xen_driver.c (xenUnifiedDriver): Likewise. * src/xenapi/xenapi_driver.c (xenapiDriver): Likewise. --- Pretty mechanical. Yup, ACK ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ 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 05/12] vcpu: implement the public APIs
On Wed, Sep 29, 2010 at 06:02:09PM -0600, Eric Blake wrote:
* src/libvirt.c (virDomainSetVcpusFlags, virDomainGetVcpusFlags):
New functions.
---
Pretty mechanical.
src/libvirt.c | 125 +++-
1 files changed, 122 insertions(+), 3 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index ca383ba..3a29d70 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5111,13 +5111,132 @@ error:
}
/**
+ * virDomainSetVcpusFlags:
+ * @domain: pointer to domain object, or NULL for Domain0
+ * @nvcpus: the new number of virtual CPUs for this domain, must be at least
1
+ * @flags: an OR'ed set of virDomainVcpuFlags
+ *
+ * Dynamically change the number of virtual CPUs used by the domain.
+ * Note that this call may fail if the underlying virtualization hypervisor
+ * does not support it or if growing the number is arbitrary limited.
+ * This function requires privileged access to the hypervisor.
+ *
+ * @flags must include VIR_DOMAIN_VCPU_ACTIVE to affect a running
+ * domain (which will fail if domain is not active), or
+ * VIR_DOMAIN_VCPU_PERSISTENT to affect the next boot via the XML
+ * description of the domain. Both flags may be set.
+ *
+ * If @flags includes VIR_DOMAIN_VCPU_MAXIMUM, then
+ * VIR_DOMAIN_VCPU_ACTIVE must be clear, and only the maximum virtual
+ * CPU limit is altered; generally, this value must be less than or
+ * equal to virConnectGetMaxVcpus(). Otherwise, this call affects the
+ * current virtual CPU limit, which must be less than or equal to the
+ * maximum limit.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
+
+int
+virDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus,
+ unsigned int flags)
+{
+virConnectPtr conn;
+DEBUG(domain=%p, nvcpus=%u, flags=%u, domain, nvcpus, flags);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
+virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+virDispatchError(NULL);
+return (-1);
+}
+if (domain-conn-flags VIR_CONNECT_RO) {
+virLibDomainError(domain, VIR_ERR_OPERATION_DENIED, __FUNCTION__);
+goto error;
+}
+
+if (nvcpus 1) {
+virLibDomainError(domain, VIR_ERR_INVALID_ARG, __FUNCTION__);
+goto error;
+}
+conn = domain-conn;
+
+if (conn-driver-domainSetVcpusFlags) {
+int ret;
+ret = conn-driver-domainSetVcpusFlags (domain, nvcpus, flags);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(domain-conn);
+return -1;
+}
+
+/**
+ * virDomainGetVcpusFlags:
+ * @domain: pointer to domain object, or NULL for Domain0
+ * @flags: an OR'ed set of virDomainVcpuFlags
+ *
+ * Query the number of virtual CPUs used by the domain. Note that
+ * this call may fail if the underlying virtualization hypervisor does
+ * not support it. This function requires privileged access to the
+ * hypervisor.
+ *
+ * @flags must include either VIR_DOMAIN_VCPU_ACTIVE to query a
+ * running domain (which will fail if domain is not active), or
+ * VIR_DOMAIN_VCPU_PERSISTENT to query the XML description of the
+ * domain. It is an error to set both flags.
+ *
+ * If @flags includes VIR_DOMAIN_VCPU_MAXIMUM, then the maximum
+ * virtual CPU limit is queried. Otherwise, this call queries the
+ * current virtual CPU limit.
+ *
+ * Returns 0 in case of success, -1 in case of failure.
+ */
+
+int
+virDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
+{
+virConnectPtr conn;
+DEBUG(domain=%p, flags=%u, domain, flags);
+
+virResetLastError();
+
+if (!VIR_IS_CONNECTED_DOMAIN(domain)) {
+virLibDomainError(NULL, VIR_ERR_INVALID_DOMAIN, __FUNCTION__);
+virDispatchError(NULL);
+return (-1);
+}
+
+conn = domain-conn;
+
+if (conn-driver-domainGetVcpusFlags) {
+int ret;
+ret = conn-driver-domainGetVcpusFlags (domain, flags);
+if (ret 0)
+goto error;
+return ret;
+}
+
+virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+virDispatchError(domain-conn);
+return -1;
+}
+
+/**
* virDomainPinVcpu:
* @domain: pointer to domain object, or NULL for Domain0
* @vcpu: virtual CPU number
* @cpumap: pointer to a bit map of real CPUs (in 8-bit bytes) (IN)
- * Each bit set to 1 means that corresponding CPU is usable.
- * Bytes are stored in little-endian order: CPU0-7, 8-15...
- * In each byte, lowest CPU number is least significant bit.
+ * Each bit set to 1 means that corresponding CPU is usable.
+ * Bytes are stored in little-endian order: CPU0-7, 8-15...
+ * In each byte, lowest CPU number is least significant bit.
* @maplen:
Re: [libvirt] [PATCH 06/12] vcpu: implement the remote protocol
On Wed, Sep 29, 2010 at 06:02:10PM -0600, Eric Blake wrote:
* daemon/remote.c (remoteDispatchDomainSetVcpusFlags)
(remoteDispatchDomainGetVcpusFlags): New functions.
* src/remote/remote_driver.c (remoteDomainSetVcpusFlags)
(remoteDomainGetVcpusFlags, remote_driver): Client side
serialization.
* src/remote/remote_protocol.x
(remote_domain_set_vcpus_flags_args)
(remote_domain_get_vcpus_flags_args)
(remote_domain_get_vcpus_flags_ret)
(REMOTE_PROC_DOMAIN_SET_VCPUS_FLAGS)
(REMOTE_PROC_DOMAIN_GET_VCPUS_FLAGS): Define wire format.
* daemon/remote_dispatch_args.h: Regenerate.
* daemon/remote_dispatch_prototypes.h: Likewise.
* daemon/remote_dispatch_table.h: Likewise.
* src/remote/remote_protocol.c: Likewise.
* src/remote/remote_protocol.h: Likewise.
* src/remote_protocol-structs: Likewise.
[...]
--- a/src/remote/remote_protocol.x
+++ b/src/remote/remote_protocol.x
@@ -728,6 +728,21 @@ struct remote_domain_set_vcpus_args {
int nvcpus;
};
+struct remote_domain_set_vcpus_flags_args {
+remote_nonnull_domain dom;
+unsigned int nvcpus;
+unsigned int flags;
+};
+
+struct remote_domain_get_vcpus_flags_args {
+remote_nonnull_domain dom;
+unsigned int flags;
+};
+
+struct remote_domain_get_vcpus_flags_ret {
+int num;
+};
+
struct remote_domain_pin_vcpu_args {
remote_nonnull_domain dom;
int vcpu;
@@ -2020,7 +2035,9 @@ enum remote_procedure {
REMOTE_PROC_DOMAIN_SNAPSHOT_DELETE = 193,
REMOTE_PROC_DOMAIN_GET_BLOCK_INFO = 194,
REMOTE_PROC_DOMAIN_EVENT_IO_ERROR_REASON = 195,
-REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS = 196
+REMOTE_PROC_DOMAIN_CREATE_WITH_FLAGS = 196,
+REMOTE_PROC_DOMAIN_SET_VCPUS_FLAGS = 197,
+REMOTE_PROC_DOMAIN_GET_VCPUS_FLAGS = 198
/*
* Notice how the entries are grouped in sets of 10 ?
that's the core part and that looks fine,
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
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 07/12] vcpu: make old API trivially wrap to new API
On Wed, Sep 29, 2010 at 06:02:11PM -0600, Eric Blake wrote:
* src/esx/esx_driver.c (esxDomainSetVcpus, escDomainGetMaxVpcus):
Move guts...
(esxDomainSetVcpusFlags, esxDomainGetVcpusFlags): ...to new
functions.
(esxDriver): Trivially support the new API.
* src/openvz/openvz_driver.c (openvzDomainSetVcpus)
(openvzDomainSetVcpusFlags, openvzDomainGetMaxVcpus)
(openvzDomainGetVcpusFlags, openvzDriver): Likewise.
* src/phyp/phyp_driver.c (phypDomainSetCPU)
(phypDomainSetVcpusFlags, phypGetLparCPUMAX)
(phypDomainGetVcpusFlags, phypDriver): Likewise.
* src/qemu/qemu_driver.c (qemudDomainSetVcpus)
(qemudDomainSetVcpusFlags, qemudDomainGetMaxVcpus)
(qemudDomainGetVcpusFlags, qemuDriver): Likewise.
* src/test/test_driver.c (testSetVcpus, testDomainSetVcpusFlags)
(testDomainGetMaxVcpus, testDomainGetVcpusFlags, testDriver):
Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainSetVcpus)
(vboxDomainSetVcpusFlags, virDomainGetMaxVcpus)
(virDomainGetVcpusFlags, virDriver): Likewise.
* src/xen/xen_driver.c (xenUnifiedDomainSetVcpus)
(xenUnifiedDomainSetVcpusFlags, xenUnifiedDomainGetMaxVcpus)
(xenUnifiedDomainGetVcpusFlags, xenUnifiedDriver): Likewise.
* src/xenapi/xenapi_driver.c (xenapiDomainSetVcpus)
(xenapiDomainSetVcpusFlags, xenapiDomainGetMaxVcpus)
(xenapiDomainGetVcpusFlags, xenapiDriver): Likewise.
(xenapiError): New helper macro.
---
Long, but consistent - anywhere the old API exists, this implements
the new API, makes the old one a wrapper around the new, and makes
the new API fail for any flag combinations not yet implemented.
src/esx/esx_driver.c | 32 +++---
src/openvz/openvz_driver.c | 34 +---
src/phyp/phyp_driver.c | 32 ---
src/qemu/qemu_driver.c | 38 +---
src/test/test_driver.c | 36 ++---
src/vbox/vbox_tmpl.c | 36 +++---
src/xen/xen_driver.c | 34 ++---
src/xenapi/xenapi_driver.c | 60
++--
8 files changed, 263 insertions(+), 39 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 1db3a90..3d13d74 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2382,7 +2382,8 @@ esxDomainGetInfo(virDomainPtr domain, virDomainInfoPtr
info)
static int
-esxDomainSetVcpus(virDomainPtr domain, unsigned int nvcpus)
+esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus,
+ unsigned int flags)
{
int result = -1;
esxPrivate *priv = domain-conn-privateData;
@@ -2392,6 +2393,11 @@ esxDomainSetVcpus(virDomainPtr domain, unsigned int
nvcpus)
esxVI_ManagedObjectReference *task = NULL;
esxVI_TaskInfoState taskInfoState;
+if (flags != VIR_DOMAIN_VCPU_ACTIVE) {
+ESX_ERROR(VIR_ERR_INVALID_ARG, _(unsupported flags: (0x%x)),
flags);
+return -1;
+}
+
if (nvcpus 1) {
ESX_ERROR(VIR_ERR_INVALID_ARG, %s,
_(Requested number of virtual CPUs must at least be 1));
@@ -2451,15 +2457,26 @@ esxDomainSetVcpus(virDomainPtr domain, unsigned int
nvcpus)
}
that error catching will end up being preempted at the main entry point but
that's fine
[...]
diff --git a/src/xenapi/xenapi_driver.c b/src/xenapi/xenapi_driver.c
index f57759a..a5690a0 100644
--- a/src/xenapi/xenapi_driver.c
+++ b/src/xenapi/xenapi_driver.c
@@ -40,6 +40,11 @@
#include xenapi_driver_private.h
#include xenapi_utils.h
+#define VIR_FROM_THIS VIR_FROM_XENAPI
+
+#define xenapiError(code, ...)\
+virReportErrorHelper(NULL, VIR_FROM_THIS, code, __FILE__, \
+ __FUNCTION__, __LINE__, __VA_ARGS__)
ah, maybe we should check that module for other uses of this, but it's
independant from this patch
Looks fine, actual implementation for QEmu coming in a following patch,
ACK
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
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 08/12] vcpu: support maxvcpu in domain_conf
On Wed, Sep 29, 2010 at 06:02:12PM -0600, Eric Blake wrote:
* src/conf/domain_conf.h (_virDomainDef): Adjust vcpus to unsigned
short, to match virDomainGetInfo limit. Add maxvcpus member.
[...]
Two tightly-related changes. One: virDomainGetInfo implicitly limits
vcpus to a 16-bit number; so there's no need to pretend otherwise
through the rest of the code.
well, yes and no, in a few years we may look ridiculous, but it would
be good to have the new APIs cleared up from that limitation.
Two: add a new maxvcpus member, but
for now, ensure that all domains treat vcpus == maxvcpus at all
times (domains that support hot-unplugging vcpus will be changed
in later patches).
[...]
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -4179,6 +4179,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr
caps,
int i, n;
long id = -1;
virDomainDefPtr def;
+unsigned long count;
if (VIR_ALLOC(def) 0) {
virReportOOMError();
@@ -4244,8 +4245,27 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr
caps,
if (node)
def-hugepage_backed = 1;
-if (virXPathULong(string(./vcpu[1]), ctxt, def-vcpus) 0)
-def-vcpus = 1;
+if (virXPathULong(string(./vcpu[1]), ctxt, count) 0)
+def-maxvcpus = 1;
+else {
+def-maxvcpus = count;
+if (def-maxvcpus != count || count == 0) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _(invalid maxvcpus %lu), count);
+goto error;
+}
+}
Hum, virXPathULong will return -2 for an non ULong format, and we
discard the error by just setting up maxvcpus = 1 silently but on the
other hand we make a fuss about 0 being provided :-)
If we start raising an error on invalid values maybe it should be
done for both (-2 need to be checked)
but not a big deal.
+if (virXPathULong(string(./vcpu[1]/@current), ctxt, count) 0)
+def-vcpus = def-maxvcpus;
+else {
+def-vcpus = count;
+if (def-vcpus != count || count == 0 || def-maxvcpus count) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _(invalid current vcpus %lu), count);
+goto error;
+}
+}
same here
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
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 08/12] vcpu: support maxvcpu in domain_conf
On Thu, Sep 30, 2010 at 01:27:27PM -0600, Eric Blake wrote:
[self-review]
On 09/29/2010 06:02 PM, Eric Blake wrote:
Two tightly-related changes. One: virDomainGetInfo implicitly limits
vcpus to a 16-bit number; so there's no need to pretend otherwise
through the rest of the code. Two: add a new maxvcpus member, but
for now, ensure that all domains treat vcpus == maxvcpus at all
times (domains that support hot-unplugging vcpus will be changed
in later patches).
@@ -4244,8 +4245,27 @@ static virDomainDefPtr
virDomainDefParseXML(virCapsPtr caps,
if (node)
def-hugepage_backed = 1;
-if (virXPathULong(string(./vcpu[1]), ctxt,def-vcpus) 0)
-def-vcpus = 1;
+if (virXPathULong(string(./vcpu[1]), ctxt,count) 0)
+def-maxvcpus = 1;
+else {
+def-maxvcpus = count;
+if (def-maxvcpus != count || count == 0) {
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _(invalid maxvcpus %lu), count);
Should this be VIR_ERR_XML_ERROR instead?
Ah, yes :-)
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
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 09/12] vcpu: add virsh support
On Wed, Sep 29, 2010 at 06:02:13PM -0600, Eric Blake wrote:
* tools/virsh.c (cmdSetvcpus): Add new flags. Let invalid
commands through to driver, to ease testing of hypervisor argument
validation.
(cmdVcpucount): New command.
(commands): Add new command.
* tools/virsh.pod (setvcpus, vcpucount): Document new behavior.
---
I know - the typical API addition sequence adds driver support
first and then virsh support. I can rearrange the patch order
if desired.
Nahh, fine, actually the entry points are there, so really there
is no problem !
[...]
/*
+ * vcpucount command
+ */
+static const vshCmdInfo info_vcpucount[] = {
+{help, N_(domain vcpu counts)},
+{desc, N_(Returns the number of domain virtual CPUs.)},
Ouch Returns the number of virtual CPUs used by the domain. would
be clearer I think
[...]
+if (maximum + current + persistent + active == 1) {
+vshError(ctl,
+ _(when using --%s, either --%s or --%s must be specified),
+ maximum ? maximum : current ? current
+ : persistent ? persistent : active,
+ maximum + current ? persistent : maximum,
+ maximum + current ? active : current);
Ouch, headache :-) but but looks right
Okay, code ended up being a bit more complex than I expected but that's
due to the various options and the fallback to old APIs, fine,
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
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 07/12] vcpu: make old API trivially wrap to new API
2010/9/30 Eric Blake ebl...@redhat.com:
* src/esx/esx_driver.c (esxDomainSetVcpus, escDomainGetMaxVpcus):
Move guts...
(esxDomainSetVcpusFlags, esxDomainGetVcpusFlags): ...to new
functions.
(esxDriver): Trivially support the new API.
* src/openvz/openvz_driver.c (openvzDomainSetVcpus)
(openvzDomainSetVcpusFlags, openvzDomainGetMaxVcpus)
(openvzDomainGetVcpusFlags, openvzDriver): Likewise.
* src/phyp/phyp_driver.c (phypDomainSetCPU)
(phypDomainSetVcpusFlags, phypGetLparCPUMAX)
(phypDomainGetVcpusFlags, phypDriver): Likewise.
* src/qemu/qemu_driver.c (qemudDomainSetVcpus)
(qemudDomainSetVcpusFlags, qemudDomainGetMaxVcpus)
(qemudDomainGetVcpusFlags, qemuDriver): Likewise.
* src/test/test_driver.c (testSetVcpus, testDomainSetVcpusFlags)
(testDomainGetMaxVcpus, testDomainGetVcpusFlags, testDriver):
Likewise.
* src/vbox/vbox_tmpl.c (vboxDomainSetVcpus)
(vboxDomainSetVcpusFlags, virDomainGetMaxVcpus)
(virDomainGetVcpusFlags, virDriver): Likewise.
* src/xen/xen_driver.c (xenUnifiedDomainSetVcpus)
(xenUnifiedDomainSetVcpusFlags, xenUnifiedDomainGetMaxVcpus)
(xenUnifiedDomainGetVcpusFlags, xenUnifiedDriver): Likewise.
* src/xenapi/xenapi_driver.c (xenapiDomainSetVcpus)
(xenapiDomainSetVcpusFlags, xenapiDomainGetMaxVcpus)
(xenapiDomainGetVcpusFlags, xenapiDriver): Likewise.
(xenapiError): New helper macro.
---
Long, but consistent - anywhere the old API exists, this implements
the new API, makes the old one a wrapper around the new, and makes
the new API fail for any flag combinations not yet implemented.
src/esx/esx_driver.c | 32 +++---
src/openvz/openvz_driver.c | 34 +---
src/phyp/phyp_driver.c | 32 ---
src/qemu/qemu_driver.c | 38 +---
src/test/test_driver.c | 36 ++---
src/vbox/vbox_tmpl.c | 36 +++---
src/xen/xen_driver.c | 34 ++---
src/xenapi/xenapi_driver.c | 60
++--
8 files changed, 263 insertions(+), 39 deletions(-)
diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 1db3a90..3d13d74 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2382,7 +2382,8 @@ esxDomainGetInfo(virDomainPtr domain, virDomainInfoPtr
info)
static int
-esxDomainSetVcpus(virDomainPtr domain, unsigned int nvcpus)
+esxDomainSetVcpusFlags(virDomainPtr domain, unsigned int nvcpus,
+ unsigned int flags)
{
int result = -1;
esxPrivate *priv = domain-conn-privateData;
@@ -2392,6 +2393,11 @@ esxDomainSetVcpus(virDomainPtr domain, unsigned int
nvcpus)
esxVI_ManagedObjectReference *task = NULL;
esxVI_TaskInfoState taskInfoState;
+ if (flags != VIR_DOMAIN_VCPU_ACTIVE) {
+ ESX_ERROR(VIR_ERR_INVALID_ARG, _(unsupported flags: (0x%x)),
flags);
+ return -1;
+ }
+
Why not use virCheckFlags here?
if (nvcpus 1) {
ESX_ERROR(VIR_ERR_INVALID_ARG, %s,
_(Requested number of virtual CPUs must at least be 1));
@@ -2451,15 +2457,26 @@ esxDomainSetVcpus(virDomainPtr domain, unsigned int
nvcpus)
}
+static int
+esxDomainSetVcpus(virDomainPtr domain, unsigned int nvcpus)
+{
+ return esxDomainSetVcpusFlags(domain, nvcpus, VIR_DOMAIN_VCPU_ACTIVE);
+}
+
static int
-esxDomainGetMaxVcpus(virDomainPtr domain)
+esxDomainGetVcpusFlags(virDomainPtr domain, unsigned int flags)
{
esxPrivate *priv = domain-conn-privateData;
esxVI_String *propertyNameList = NULL;
esxVI_ObjectContent *hostSystem = NULL;
esxVI_DynamicProperty *dynamicProperty = NULL;
+ if (flags != (VIR_DOMAIN_VCPU_ACTIVE | VIR_DOMAIN_VCPU_MAXIMUM)) {
+ ESX_ERROR(VIR_ERR_INVALID_ARG, _(unsupported flags: (0x%x)),
flags);
+ return -1;
+ }
+
virCheckFlags?
This pattern reoccurs through the rest of the patch.
Matthias
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 10/12] vcpu: improve vcpu support in qemu command line
On Wed, Sep 29, 2010 at 06:02:14PM -0600, Eric Blake wrote:
* src/qemu/qemu_conf.c (qemuParseCommandLineSmp): Distinguish
between vcpus and maxvcpus, for new enough qemu.
* tests/qemuargv2xmltest.c (mymain): Add new test.
* tests/qemuxml2argvtest.c (mymain): Likewise.
* tests/qemuxml2xmltest.c (mymain): Likewise.
* tests/qemuxml2argvdata/qemuxml2argv-smp.args: New file.
---
This manages just the command-line aspects needed for qemu 0.12+;
so far, I haven't done anything about 0.11 or earlier (on those
versions of qemu, maxvcpus and vcpus must always be equal).
src/qemu/qemu_conf.c | 13 +
tests/qemuargv2xmltest.c |2 ++
tests/qemuxml2argvdata/qemuxml2argv-smp.args |1 +
tests/qemuxml2argvtest.c |2 ++
tests/qemuxml2xmltest.c |2 ++
5 files changed, 16 insertions(+), 4 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-smp.args
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 5169e3c..f387a5d 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -3616,6 +3616,8 @@ qemuBuildSmpArgStr(const virDomainDefPtr def,
virBufferVSprintf(buf, %u, def-vcpus);
if ((qemuCmdFlags QEMUD_CMD_FLAG_SMP_TOPOLOGY)) {
ohhh, we were already loking for this option at run time ...
+if (def-vcpus != def-maxvcpus)
+virBufferVSprintf(buf, ,maxcpus=%u, def-maxvcpus);
/* sockets, cores, and threads are either all zero
* or all non-zero, thus checking one of them is enough */
if (def-cpu def-cpu-sockets) {
@@ -3628,12 +3630,12 @@ qemuBuildSmpArgStr(const virDomainDefPtr def,
virBufferVSprintf(buf, ,cores=%u, 1);
virBufferVSprintf(buf, ,threads=%u, 1);
}
-}
-if (def-vcpus != def-maxvcpus) {
+} else if (def-vcpus != def-maxvcpus) {
virBufferFreeAndReset(buf);
+// FIXME - consider hot-unplugging cpus after boot
Please no C++ like comments :-)
qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(setting current vcpu count less than maximum is
- not supported yet));
+ not supported with this QEMU binary));
return NULL;
}
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
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 07/12] vcpu: make old API trivially wrap to new API
On 10/01/2010 09:26 AM, Matthias Bolte wrote:
@@ -2392,6 +2393,11 @@ esxDomainSetVcpus(virDomainPtr domain, unsigned int
nvcpus)
esxVI_ManagedObjectReference *task = NULL;
esxVI_TaskInfoState taskInfoState;
+if (flags != VIR_DOMAIN_VCPU_ACTIVE) {
+ESX_ERROR(VIR_ERR_INVALID_ARG, _(unsupported flags: (0x%x)), flags);
+return -1;
+}
+
Why not use virCheckFlags here?
virCheckFlags is useful for the maximal set of permitted flags,
regardless of whether they are set or cleared. This instance was going
for zero semantic change, therefore we expect an exact flag set. At
which point, checking for the exact flag is easier than checking for all
supported flags and then an exact flag check.
See patch 11/12 for an example of how qemu replaces an exact flag check
with virCheckFlags when we finally add real support for all flag
combinations. The same thing can be done for esx when (and if) we
figure out how to support all flag combinations.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 11/12] vcpu: complete vcpu support in qemu driver
On Wed, Sep 29, 2010 at 06:09:21PM -0600, Eric Blake wrote:
* src/qemu/qemu_driver.c (qemudDomainSetVcpusFlags)
(qemudDomainGetVcpusFlags): Support all feasible flag
combinations.
---
Aargh - my ISP SMTP server said that 11 emails is the limit for one
session, so I have to resend the last two of my series (and figure out
how to config git to use multiple sessions to avoid that problem in
the future).
worked fine, I saw them in a batch, properly aligned,
Anyways, this is the meat of the patch series, in converting qemu
0.12+ to remember whether vcpus have been hot-unplugged below the
maximum, in order to boot that way the next time around. I'm a bit
fuzzy on whether I used all the driver internals correctly, especially
on how and when vm-newDef should be created for managing persistent
state separately from live state, so a careful review is appreciated,
and I'll probably need a v2.
[...]
-ret = qemudDomainHotplugVcpus(vm, nvcpus);
+switch (flags) {
+case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_PERSISTENT:
hum I usually indent the cases 4 spaces from the switch, minor nit
ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/
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 12/12] food for thought
On 10/01/2010 04:18 PM, Daniel Veillard wrote: I would think augmenting the sexpr should be sufficient for this but the problem is really to find out when the feature is available, and I don't know how to do this reliably either (except trying and if there is an identifiable error keep it disabled). It seems to date back to upstream change set 7222, which is old enough not to worry. Paolo -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 01/12] vcpu: improve cpuset attribute
On 10/01/2010 08:19 AM, Daniel Veillard wrote: On Wed, Sep 29, 2010 at 06:02:05PM -0600, Eric Blake wrote: Thevcpu cpuset=... attribute has been available since commit e193b5dd, but without documentation or RNG validation. * docs/schemas/domain.rng (vcpu): Further validate cpuset. * docs/formatdomain.html.in: Document it. * src/conf/domain_conf.c: Fix typos. ACK, actually independant fom the other patches, Pushed this one since it is independent, like you noticed. I'm probably going to do a v2 for the other patches, even though they've been ACKed, to show how I've folded in the nit fixups, and including a xen implementation. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] RFC: automatic setting of ip_forwarding (or not)
Currently libvirt will turn on net.ipv4.ip_forward by writing 1\n to /proc/sys/net/ipv4/ip_forward whenever a virtual network of with a forward mode of nat or route is started. This is problematic for two reasons: 1) /etc/sysctl.conf is not updated with this information, so any other process reprocessing /etc/sysctl.conf (with sysctl -a -p) will potentially turn ip forward back to 0, leaving libvirt-created virtual networks in a non-working state, and 2) it's possible the administrator turned off ip forwarding on purpose for security reasons, and our silently turning it on leaves them mistakenly believing it is still off. We've discussed a few ways of remedying this situation lately, and I thought I should summarize all the mentioned ideas, and take a poll to try and determine which way we should fix this. 1) Leave it as is. The simplest solution, but has the problems outlines above. 2) Turn it on in the same place, but do it by writing net.ipv4.ip_forward = 1 to /etc/sysctl.conf and calling sysctl -a -p. This gives us the same behavior as currently, but with the advantages that a) our change to the config is documented in /etc/sysctl.conf and b) virtual networked guests won't suddenly have their network fail when some other process runs sysconfig -a -p. However, it seems rather drastic to be turning this on every time a virtual network is started, especially without alerting the admin that this has been done. 3) Whenever a virtual network that would require ip_forward = 1 to operate properly is started (ie at libvirtd start time, and when a network is newly defined), check if it's currently on, and if not log a warning message, informing the admin that they should turn on ip_forward in sysctl.conf and reload it in order to have properly working networking. This would assure that the admin was informed of the necessity for ip_forward, but eliminate the possibility of two processes fighting over the setting of ip_forward, leaving it up to the admin to make the decision and do the right thing. On the other hand, it would prevent libvirt's networking from just working on a new install. 4) Turn ip_forward on during libvirt install. This one doesn't make sense to me, because you don't know at the time of libvirt install whether or not the installation if going to end up with any virtual networks that need forwarding. 5) Make ip_forward a tunable in /etc/libvirt/libvirtd.conf, and set it accordingly every time libvirtd is started. I don't know if this makes sense either - if you have NATed or routed virtual networks, you will need ip_forward=1 for them to work properly, and if you don't have them, you don't care, so it's really redundant. I think the important things to accomplish are: 1) Avoid having networking magically stop working when someone else reloads sysctl.conf 2) Make sure that the admin realizes that ip_forward is being turned on (or needs to be turned on). 3) If we're going to turn it on, at least don't do it if it's not needed. 4) Something else we need to consider is the ability to provision a host for proper guest networking purely through the libvirt API, and if we were to stop turning on ip_forward automatically when a network was started, that wouldn't work anymore (unless ip_forward happened to be turned on already). So, what are your opinions? (BTW, the firewall rules added for virtual networks suffer from a similar problem - because they're loaded into the kernel directly with the iptables command, there is no external record of them, and some other process reloading the firewall will flush out all libvirt's rules, leaving the guests with nonworking networking. But that discussion is a bigger one, that probably needs to go outside just libvirt, so I'll avoid that here...) -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 3/4] xen: xenXMDomain*DeviceFlags should obey all flags
xenXMDomain*DeviceFlags() silently ignores requests to modify live
configuration of an active guest while still touching its persistent
configuration.
---
src/xen/xm_internal.c | 14 --
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index b9cb4c3..fcc9378 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -2935,8 +2935,13 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain, const
char *xml,
if (domain-conn-flags VIR_CONNECT_RO)
return -1;
-if (domain-id != -1 !(flags VIR_DOMAIN_DEVICE_MODIFY_CONFIG))
+
+if ((flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) ||
+(domain-id != -1 (flags VIR_DOMAIN_DEVICE_MODIFY_CURRENT))) {
+xenXMError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Xm driver only supports modifying persistent config));
return -1;
+}
priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
xenUnifiedLock(priv);
@@ -3026,8 +3031,13 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain, const
char *xml,
if (domain-conn-flags VIR_CONNECT_RO)
return -1;
-if (domain-id != -1 !(flags VIR_DOMAIN_DEVICE_MODIFY_CONFIG))
+
+if ((flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) ||
+(domain-id != -1 (flags VIR_DOMAIN_DEVICE_MODIFY_CURRENT))) {
+xenXMError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Xm driver only supports modifying persistent config));
return -1;
+}
priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
xenUnifiedLock(priv);
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 2/4] xen: Fix logic bug in xenDaemon*DeviceFlags
---
src/xen/xend_internal.c | 15 +--
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 1318bd4..4fba6af 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3904,8 +3904,9 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const
char *xml,
/* Xen only supports modifying both live and persistent config if
* xendConfigVersion = 3
*/
-if (flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
- VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
+if (priv-xendConfigVersion = 3
+(flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
+ VIR_DOMAIN_DEVICE_MODIFY_CONFIG))) {
virXendError(VIR_ERR_OPERATION_INVALID, %s,
_(Xend only supports modifying both live and
persistent config));
@@ -4043,8 +4044,9 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const
char *xml,
/* Xen only supports modifying both live and persistent config if
* xendConfigVersion = 3
*/
-if (flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
- VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
+if (priv-xendConfigVersion = 3
+(flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
+ VIR_DOMAIN_DEVICE_MODIFY_CONFIG))) {
virXendError(VIR_ERR_OPERATION_INVALID, %s,
_(Xend only supports modifying both live and
persistent config));
@@ -4154,8 +4156,9 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, const
char *xml,
/* Xen only supports modifying both live and persistent config if
* xendConfigVersion = 3
*/
-if (flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
- VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
+if (priv-xendConfigVersion = 3
+(flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
+ VIR_DOMAIN_DEVICE_MODIFY_CONFIG))) {
virXendError(VIR_ERR_OPERATION_INVALID, %s,
_(Xend only supports modifying both live and
persistent config));
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 4/4] xen: Fix virDomain{At,De}tachDevice
According to API documentation virDomain{At,De}tachDevice calls are
supposed to only work on active guests for device hotplug. For anything
beyond that, their *Flags variants have to be used.
Despite the variant which was acked on libvirt mailing list
(https://www.redhat.com/archives/libvir-list/2010-January/msg00385.html)
commit ed9c14a7ef86d7a45a6d57cbfee5410fca428633 (by Jim Fehlig)
introduced automagic behavior of these API calls for xen driver. Since
January, these calls always change persistent configuration of a guest
and if the guest is currently active, they also hot(un)plug the device.
That change didn't follow API documentation and also broke device
hot(un)plug for older xend implementations which do not support changing
persistent configuration of a guest and hot(un)plugging in one step.
This patch should not break anything for active guests. On the other
hand, changing inactive guests is not supported any more.
---
src/xen/xen_driver.c | 24 ++--
1 files changed, 18 insertions(+), 6 deletions(-)
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 56ba41b..e9eeab9 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -1437,10 +1437,16 @@ xenUnifiedDomainAttachDevice (virDomainPtr dom, const
char *xml)
{
GET_PRIVATE(dom-conn);
int i;
-unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
+unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE;
-if (dom-id = 0)
-flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
+/*
+ * HACK: xend with xendConfigVersion = 3 does not support changing live
+ * config without touching persistent config, we add the extra flag here
+ * to make this API work
+ */
+if (priv-opened[XEN_UNIFIED_XEND_OFFSET]
+priv-xendConfigVersion = 3)
+flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
if (priv-opened[i] drivers[i]-domainAttachDeviceFlags
@@ -1470,10 +1476,16 @@ xenUnifiedDomainDetachDevice (virDomainPtr dom, const
char *xml)
{
GET_PRIVATE(dom-conn);
int i;
-unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
+unsigned int flags = VIR_DOMAIN_DEVICE_MODIFY_LIVE;
-if (dom-id = 0)
-flags |= VIR_DOMAIN_DEVICE_MODIFY_LIVE;
+/*
+ * HACK: xend with xendConfigVersion = 3 does not support changing live
+ * config without touching persistent config, we add the extra flag here
+ * to make this API work
+ */
+if (priv-opened[XEN_UNIFIED_XEND_OFFSET]
+priv-xendConfigVersion = 3)
+flags |= VIR_DOMAIN_DEVICE_MODIFY_CONFIG;
for (i = 0; i XEN_UNIFIED_NR_DRIVERS; ++i)
if (priv-opened[i] drivers[i]-domainDetachDeviceFlags
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH 1/4] xen: Make xenDaemon*DeviceFlags errors less confusing
When a user calls to virDomain{Attach,Detach,Update}DeviceFlags() with
flags == VIR_DOMAIN_DEVICE_MODIFY_LIVE on an inactive guest running on
an old Xen hypervisor (such as RHEL-5) xend_internal driver reports:
Xend version does not support modifying persistent config
which is pretty confusing since no-one requested to modify persistent
config.
---
src/xen/xend_internal.c | 36 ++--
1 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index fce0233..1318bd4 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3878,6 +3878,12 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const
char *xml,
priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
if (domain-id 0) {
+/* Cannot modify live config if domain is inactive */
+if (flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
+virXendError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Cannot modify live config if domain is inactive));
+return -1;
+}
/* If xendConfigVersion 3 only live config can be changed */
if (priv-xendConfigVersion 3) {
virXendError(VIR_ERR_OPERATION_INVALID, %s,
@@ -3885,12 +3891,6 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const
char *xml,
persistent config));
return -1;
}
-/* Cannot modify live config if domain is inactive */
-if (flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
-virXendError(VIR_ERR_OPERATION_INVALID, %s,
- _(Cannot modify live config if domain is inactive));
-return -1;
-}
} else {
/* Only live config can be changed if xendConfigVersion 3 */
if (priv-xendConfigVersion 3
@@ -4017,6 +4017,12 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const
char *xml,
priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
if (domain-id 0) {
+/* Cannot modify live config if domain is inactive */
+if (flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
+virXendError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Cannot modify live config if domain is inactive));
+return -1;
+}
/* If xendConfigVersion 3 only live config can be changed */
if (priv-xendConfigVersion 3) {
virXendError(VIR_ERR_OPERATION_INVALID, %s,
@@ -4024,12 +4030,6 @@ xenDaemonUpdateDeviceFlags(virDomainPtr domain, const
char *xml,
persistent config));
return -1;
}
-/* Cannot modify live config if domain is inactive */
-if (flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
-virXendError(VIR_ERR_OPERATION_INVALID, %s,
- _(Cannot modify live config if domain is inactive));
-return -1;
-}
} else {
/* Only live config can be changed if xendConfigVersion 3 */
if (priv-xendConfigVersion 3
@@ -4128,6 +4128,12 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, const
char *xml,
priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
if (domain-id 0) {
+/* Cannot modify live config if domain is inactive */
+if (flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
+virXendError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Cannot modify live config if domain is inactive));
+return -1;
+}
/* If xendConfigVersion 3 only live config can be changed */
if (priv-xendConfigVersion 3) {
virXendError(VIR_ERR_OPERATION_INVALID, %s,
@@ -4135,12 +4141,6 @@ xenDaemonDetachDeviceFlags(virDomainPtr domain, const
char *xml,
persistent config));
return -1;
}
-/* Cannot modify live config if domain is inactive */
-if (flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
-virXendError(VIR_ERR_OPERATION_INVALID, %s,
- _(Cannot modify live config if domain is inactive));
-return -1;
-}
} else {
/* Only live config can be changed if xendConfigVersion 3 */
if (priv-xendConfigVersion 3
--
1.7.3.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virt-install feature request
So virt-install is a really nice piece of software, but it's a bit too complicated for our use case :). We would want some mode of it to simply generate XML and go home :). The use case is that we have a script which carries out a number of other actions in addition to creating a guest, and creating a guest and putting an OS on it are two separate steps. It would also be nice to have some command line interface to modify a guest's boot selection, for example, changing from PXE to local disk. Even nicer would be a concept of boot order - we have a setup where the first normal boot order is PXE, and if that fails (which it should a majority of the time) then go to local disk. -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] indentation (was: [PATCH 11/12] vcpu: complete vcpu support in qemu driver)
On 10/01/2010 09:31 AM, Daniel Veillard wrote:
-ret = qemudDomainHotplugVcpus(vm, nvcpus);
+switch (flags) {
+case VIR_DOMAIN_VCPU_MAXIMUM | VIR_DOMAIN_VCPU_PERSISTENT:
hum I usually indent the cases 4 spaces from the switch, minor nit
I just used what emacs gave by default, after applying the suggested
~/.emacs settings from HACKING. In that mode, emacs puts the labels
flush with the switch. The libvirt code base has a decent mix of both
styles, but favors my style over yours (don't ask me how much time I
wasted trying to figure out that sed script):
$ git grep -A1 switch src \
| sed -n 's/:/-/;/switch/{N;/^\(.*\)switch.*\n\1case/p}' | wc -l
350
$ git grep -A1 switch src \
| sed -n 's/:/-/;/switch/{N;/^\(.*\)switch.*\n\1 *case/p}' | wc -l
96
Speaking of indentation nits, are we any closer to Jim's idea of using
an automated indent-like tool for setting on a consistent formatting style?
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 03/12] vcpu: add new public API
On 10/01/2010 10:13 AM, Matthias Bolte wrote: However, in implementing things, I'm wondering if I should use the names: VIR_DOMAIN_VCPU_CONFIG (instead of VIR_DOMAIN_VCPU_PERSISTENT) VIR_DOMAIN_VCPU_LIVE (instead of VIR_DOMAIN_VCPU_ACTIVE) to match virDomainDeviceModifyFlags, where _CONFIG and _LIVE have the same semantics of setting one or both aspects of a domain. If you want to match the semantics of _CONFIG and _LIVE from virDomainDeviceModifyFlags then you need to change the ESX driver differently in patch 7/12. If I understand the semantics of virDomainDeviceModifyFlags correctly, then _LIVE is only affecting the runtime state of the domain and changes done with the _LIVE flag only are lost on domain restart. In the same way _CONFIG only affects the persistent config and not the runtime state. Yes, that was the usage pattern I had envisioned, to allow for domains like qemu where you can make a live-only change. With ESX there is no such thing as a distinct runtime state. Changes to the number of vCPUs affect the runtime state and the persistent config at the same time. And, in playing with things a bit more, this is what I see pre-patch on Fedora 14 (# for host, $ for guest): # virsh edit fedora_14 # virsh dumpxml fedora_14 | grep vcpu vcpu2/vcpu # virsh start fedora_14 Domain fedora_14 started # virsh setvcpus fedora_14 3 [hmm, trying to exceed the max killed my guest - not very nice] # virsh setvcpus fedora_14 1 error: Requested operation is not valid: domain is not running [it currently requires a live domain] # virsh dumpxml fedora_14 | grep vcpu vcpu3/vcpu [but it changed the configured value] # virsh start fedora_14 Domain fedora_14 started # virsh setvcpus fedora_14 2 # virsh dumpxml fedora_14 | grep vcpu vcpu2/vcpu $ nproc 3 [hmm - the live value is still 3] $ shutdown -h now # virsh start fedora_14 Domain fedora_14 started $ nproc 2 [yep - definitely a persistent setting, and I couldn't figure how it is supposed to be live, since I could not see qemuMonitorSetCPU having a visible effect on nproc(1) in the guest?] Currently you altered the ESX driver in patch 7/12 to esxDomainSetVcpus = esxDomainSetVcpusFlags(VIR_DOMAIN_VCPU_ACTIVE) But this should actually use _LIVE | _CONFIG as flags to match what ESX can do only. I'm starting to think that should be the case in general: virDomainSetVcpus should map to virDomainSetVcpusFlags(VIR_DOMAIN_VCPU_LIVE|VIR_DOMAIN_VCPU_CONFIG), and the new behavior becomes that of specifying live-only or config-only. There may yet be some logic bugs for some drivers, and certainly this argues for additions to the TCK on hot-plugged vcpu behavior. At any rate, I really need to post a v2 series that tries to be more careful with my handling of live vs. configuration. Also in 5/12 you state that trying to change the vCPU count of an inactive domain using the _LIVE flag should fail. The implementation in the ESX driver in 7/12 doesn't match this either. If the domain is inactive, then _LIVE can't possibly change anything. I could probably agree that it makes more sense to have _CONFIG|_LIVE silently ignore _LIVE when a domain is inactive rather than erroring out, especially given the argument that the existing setVcpus command without flags should probably be using _CONFIG|_LIVE; except that the qemu example above proved that the existing setVcpus errors out if the domain is not live. And I still think that _LIVE in isolation on an inactive domain should error out (the _CONFIG|_LIVE combination is different, because _CONFIG can still do useful work). esxDomainSetVcpus needs to call esxDomainSetVcpusFlags with _CONFIG and add _LIVE to the set of flags when the domain is active. Likewise esxDomainSetVcpusFlags needs to enforce that _CONFIG is always set and that _LIVE is set iff the domain is active. The same might apply to other drivers, I didn't check this yet. Matthias -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: automatic setting of ip_forwarding (or not)
On 10/01/2010 12:46 PM, Laine Stump wrote: 3) Whenever a virtual network that would require ip_forward = 1 to operate properly is started (ie at libvirtd start time, and when a network is newly defined), check if it's currently on, and if not log a warning message, informing the admin that they should turn on ip_forward in sysctl.conf and reload it in order to have properly working networking. This would assure that the admin was informed of the necessity for ip_forward, but eliminate the possibility of two processes fighting over the setting of ip_forward, leaving it up to the admin to make the decision and do the right thing. On the other hand, it would prevent libvirt's networking from just working on a new install. Option 3 is consistent with what we just checked in for nwfilter vs. /proc/sys/net/bridge/bridge-nf-call-ip[6]tables in commit 570d040435. On the surface, I'm liking that option best - tell the user that we can't do what they asked because it requires them to make a conscious admin decision; even though it unfortunately doesn't play nicely with out-of-the-box installation (and that matters more for user attitudes than anything else, in my experience). 4) Turn ip_forward on during libvirt install. This one doesn't make sense to me, because you don't know at the time of libvirt install whether or not the installation if going to end up with any virtual networks that need forwarding. Now, thinking a bit outside the box: is it possible to create a libvirt-network package, whose sole purpose is to turn on the ip_forward setting at install time? That is, the main libvirt package doesn't touch the setting, but an optional add-on package does; that way, users can choose whether to install one or both packages, depending on their intended usage patterns. In other words, I think point 4 via split packaging solves this nicely. I think the important things to accomplish are: 1) Avoid having networking magically stop working when someone else reloads sysctl.conf Agreed. Point 3 meets this, but by putting the burden on the sysadmin. Point 4 also meets it by actually changing the persistent config, if the optional package is installed. 2) Make sure that the admin realizes that ip_forward is being turned on (or needs to be turned on). Point 3 meets this via error message; point 4 meets this by explicitly requesting the extra package. 3) If we're going to turn it on, at least don't do it if it's not needed. Point 3 meets this by leaving it up to the sysadmin; point 4 meets this by leaving the optional package out. 4) Something else we need to consider is the ability to provision a host for proper guest networking purely through the libvirt API, and if we were to stop turning on ip_forward automatically when a network was started, that wouldn't work anymore (unless ip_forward happened to be turned on already). Not sure how best to address this through just libvirt API. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: automatic setting of ip_forwarding (or not)
Uh huh, hi? On 10/01/2010 08:46 PM, Laine Stump wrote: [...] to /etc/sysctl.conf and calling sysctl -a -p. This gives us the same behavior as currently, but with the advantages that a) our change to the config is documented in /etc/sysctl.conf and b) virtual networked guests won't suddenly have their network fail when some other process runs sysconfig -a -p. You've got me a bit confused here, because what exactly is supposed # sysctl -a -p; do? I mean, what kind of magic? I've tried this and if I turn on net.ipv4.ip_forward to '1' and run # sysctl -a -p; it stays on, unless defined as 0 in /etc/sysctl.conf (and sysctl run only with -p, not -a AND -p). And if it's defined as 0 there, then there must be reason for being so. Also, why on Earth would you have every sysctl parameter in /etc/sysctl.conf unless you're not happy with default ones? Imho this file is for fine tuning, thus ~ include things what I don't like or want changed, but not every possible kernel option which I might or might not want to change. Because then there is no surprise for such odd behavior. Also, the outlined problem here sounds more like left hand doesn't know what's doing right hand or I just didn't get your point. I'm also missing the previous discussion about the problem, if there was any. I mean, your e-mail comes out of the blue sky without any previous reference. I hope this doesn't sound too much jerk-ish, but I kinda don't like too much what I'm reading here. Have a nice weekend and thank you for your (or anyone's) reply on this one. Zdenek -- Zdenek Styblik Net/Linux admin OS TurnovFree.net email: sty...@turnovfree.net jabber: sty...@jabber.turnovfree.net -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/4] xen: Make xenDaemon*DeviceFlags errors less confusing
On 10/01/2010 02:09 PM, Jiri Denemark wrote:
When a user calls to virDomain{Attach,Detach,Update}DeviceFlags() with
flags == VIR_DOMAIN_DEVICE_MODIFY_LIVE on an inactive guest running on
an old Xen hypervisor (such as RHEL-5) xend_internal driver reports:
Xend version does not support modifying persistent config
which is pretty confusing since no-one requested to modify persistent
config.
Hmm - given the recent discussion on vcpus (which is probably what made
you look at this, right?)...
---
src/xen/xend_internal.c | 36 ++--
1 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index fce0233..1318bd4 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3878,6 +3878,12 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const
char *xml,
priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
if (domain-id 0) {
+/* Cannot modify live config if domain is inactive */
+if (flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) {
+virXendError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Cannot modify live config if domain is inactive));
+return -1;
+}
Should we always error out if _LIVE and inactive, or should we
special-case _CONFIG|_LIVE by silently ignoring the _LIVE flag on
inactive domains?
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] xen: Fix logic bug in xenDaemon*DeviceFlags
On 10/01/2010 02:09 PM, Jiri Denemark wrote:
---
src/xen/xend_internal.c | 15 +--
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 1318bd4..4fba6af 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3904,8 +3904,9 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain, const
char *xml,
/* Xen only supports modifying both live and persistent config if
* xendConfigVersion= 3
*/
-if (flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
- VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
+if (priv-xendConfigVersion= 3
+(flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
+ VIR_DOMAIN_DEVICE_MODIFY_CONFIG))) {
virXendError(VIR_ERR_OPERATION_INVALID, %s,
_(Xend only supports modifying both live and
persistent config));
The comment says:
_LIVE|_CONFIG is supported only if version=3
logically, this tells me nothing about version 3, nor about setting
one but not both flags.
Meanwhile, the code says:
if version =3, _LIVE|_CONFIG is the only supported combo
Are we sure this is the right change? What happens with version 3?
It seems like we need a 12-way table (in fact, this is pretty much what
I ended up resorting to with my vcpus stuff). Here's my shot at it,
from reading the comments (but not actually testing it); once we fix
this attempt to an actual table, then I can answer whether the code
matches the table.
_LIVE _CONFIG _LIVE|_CONFIG
xen 2,running good unsupported unsupported
xen 2,inactiveerror good error or silently good
xen 3,running good good good
xen 3,inactiveerror good error or silently good
where the 'error or silently good' depends on our decision of whether an
inactive domain should silently ignore _LIVE.
--
Eric Blake ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/4] xen: xenXMDomain*DeviceFlags should obey all flags
Jiri Denemark wrote:
xenXMDomain*DeviceFlags() silently ignores requests to modify live
configuration of an active guest while still touching its persistent
configuration.
---
src/xen/xm_internal.c | 14 --
1 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/src/xen/xm_internal.c b/src/xen/xm_internal.c
index b9cb4c3..fcc9378 100644
--- a/src/xen/xm_internal.c
+++ b/src/xen/xm_internal.c
@@ -2935,8 +2935,13 @@ xenXMDomainAttachDeviceFlags(virDomainPtr domain,
const char *xml,
if (domain-conn-flags VIR_CONNECT_RO)
return -1;
-if (domain-id != -1 !(flags VIR_DOMAIN_DEVICE_MODIFY_CONFIG))
+
+if ((flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) ||
+(domain-id != -1 (flags VIR_DOMAIN_DEVICE_MODIFY_CURRENT))) {
+xenXMError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Xm driver only supports modifying persistent config));
return -1;
+}
priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
xenUnifiedLock(priv);
@@ -3026,8 +3031,13 @@ xenXMDomainDetachDeviceFlags(virDomainPtr domain,
const char *xml,
if (domain-conn-flags VIR_CONNECT_RO)
return -1;
-if (domain-id != -1 !(flags VIR_DOMAIN_DEVICE_MODIFY_CONFIG))
+
+if ((flags VIR_DOMAIN_DEVICE_MODIFY_LIVE) ||
+(domain-id != -1 (flags VIR_DOMAIN_DEVICE_MODIFY_CURRENT))) {
+xenXMError(VIR_ERR_OPERATION_INVALID, %s,
+ _(Xm driver only supports modifying persistent config));
return -1;
+}
priv = (xenUnifiedPrivatePtr) domain-conn-privateData;
xenUnifiedLock(priv);
ACK.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] xen: Fix logic bug in xenDaemon*DeviceFlags
Eric Blake wrote:
On 10/01/2010 02:09 PM, Jiri Denemark wrote:
---
src/xen/xend_internal.c | 15 +--
1 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c
index 1318bd4..4fba6af 100644
--- a/src/xen/xend_internal.c
+++ b/src/xen/xend_internal.c
@@ -3904,8 +3904,9 @@ xenDaemonAttachDeviceFlags(virDomainPtr domain,
const char *xml,
/* Xen only supports modifying both live and persistent config if
* xendConfigVersion= 3
*/
- if (flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
- VIR_DOMAIN_DEVICE_MODIFY_CONFIG)) {
+ if (priv-xendConfigVersion= 3
+ (flags != (VIR_DOMAIN_DEVICE_MODIFY_LIVE |
+ VIR_DOMAIN_DEVICE_MODIFY_CONFIG))) {
virXendError(VIR_ERR_OPERATION_INVALID, %s,
_(Xend only supports modifying both live and
persistent config));
The comment says:
_LIVE|_CONFIG is supported only if version=3
logically, this tells me nothing about version 3, nor about setting
one but not both flags.
Meanwhile, the code says:
if version =3, _LIVE|_CONFIG is the only supported combo
Are we sure this is the right change? What happens with version 3?
It seems like we need a 12-way table (in fact, this is pretty much
what I ended up resorting to with my vcpus stuff).
Yes, good idea.
Here's my shot at it, from reading the comments (but not actually
testing it); once we fix this attempt to an actual table, then I can
answer whether the code matches the table.
_LIVE _CONFIG _LIVE|_CONFIG
xen 2,running good unsupported unsupported
xen 2,inactive error good error or silently good
xen 3,running good good good
I'm not aware of any way to _only_ hot(un)plug or _only_ change
persistent config in this case. If xend's device_create op is called on
a running domain, the device is hotplugged *and* it is added to the config.
xen 3,inactive error good error or silently good
where the 'error or silently good' depends on our decision of whether
an inactive domain should silently ignore _LIVE.
Otherwise the table looks correct. And IMO silently good is acceptable.
Jim
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nwfilter: fix memory leaks
Fixing memory leak shown by valgrind and freeing buffer in two more
places.
Signed-off-by: Stefan Berger stef...@us.ibm.com
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -1508,7 +1508,11 @@ _iptablesCreateRuleInstance(int directio
}
if (virBufferUse(prefix)) {
-virBufferVSprintf(prefix, %s, virBufferContentAndReset(buf));
+char *s = virBufferContentAndReset(buf);
+
+virBufferVSprintf(prefix, %s, s);
+
+VIR_FREE(s);
final = prefix;
@@ -1531,11 +1535,13 @@ _iptablesCreateRuleInstance(int directio
err_exit:
virBufferFreeAndReset(buf);
+virBufferFreeAndReset(prefix);
return -1;
exit_no_error:
virBufferFreeAndReset(buf);
+virBufferFreeAndReset(prefix);
return 0;
}
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: automatic setting of ip_forwarding (or not)
On 10/01/2010 04:40 PM, Eric Blake wrote: On 10/01/2010 12:46 PM, Laine Stump wrote: 3) Whenever a virtual network that would require ip_forward = 1 to operate properly is started (ie at libvirtd start time, and when a network is newly defined), check if it's currently on, and if not log a warning message, informing the admin that they should turn on ip_forward in sysctl.conf and reload it in order to have properly working networking. This would assure that the admin was informed of the necessity for ip_forward, but eliminate the possibility of two processes fighting over the setting of ip_forward, leaving it up to the admin to make the decision and do the right thing. On the other hand, it would prevent libvirt's networking from just working on a new install. Option 3 is consistent with what we just checked in for nwfilter vs. /proc/sys/net/bridge/bridge-nf-call-ip[6]tables in commit 570d040435. On the surface, I'm liking that option best - tell the user that we can't do what they asked because it requires them to make a conscious admin decision; even though it unfortunately doesn't play nicely with out-of-the-box installation (and that matters more for user attitudes than anything else, in my experience). The problem may be that not everyone knows how to enable forwarding, filtering, or reads log files. People are used to that networking (or filtering) works out-of-the box but then on one installation for some reason it doesn't. After some time typically one finds the answer online somewhere what needs to be configured... I guess documentation is 'key'. Maybe libvirt could have an admin mode 'somehow' that prevents it from automatically making those changes whereas the less-savvy audience gets it set up all automatically. Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [patch 5/5] nwfilter: Extend docs with information about the comment attribute
I am adding a row with information about the newly supported state attribute to each of the tables describing supported attributes of protocols. Signed-off-by: Stefan Berger stef...@us.ibm.com --- docs/formatnwfilter.html.in | 30 ++ 1 file changed, 30 insertions(+) Index: libvirt-acl/docs/formatnwfilter.html.in === --- libvirt-acl.orig/docs/formatnwfilter.html.in +++ libvirt-acl/docs/formatnwfilter.html.in @@ -748,6 +748,11 @@ tdSTRING/td tdtext with max. 256 characters/td /tr + tr + tdstate span class=since(Since 0.8.5)/span/td + tdSTRING/td + tdcomma separated list of NEW,ESTABLISHED,RELATED,INVALID or NONE/td + /tr /table p brbr @@ -843,6 +848,11 @@ tdSTRING/td tdtext with max. 256 characters/td /tr + tr + tdstate span class=since(Since 0.8.5)/span/td + tdSTRING/td + tdcomma separated list of NEW,ESTABLISHED,RELATED,INVALID or NONE/td + /tr /table p brbr @@ -927,6 +937,11 @@ tdSTRING/td tdtext with max. 256 characters/td /tr + tr + tdstate span class=since(Since 0.8.5)/span/td + tdSTRING/td + tdcomma separated list of NEW,ESTABLISHED,RELATED,INVALID or NONE/td + /tr /table p brbr @@ -1018,6 +1033,11 @@ tdSTRING/td tdtext with max. 256 characters/td /tr + tr + tdstate span class=since(Since 0.8.5)/span/td + tdSTRING/td + tdcomma separated list of NEW,ESTABLISHED,RELATED,INVALID or NONE/td + /tr /table p brbr @@ -1099,6 +1119,11 @@ tdSTRING/td tdtext with max. 256 characters/td /tr + tr + tdstate span class=since(Since 0.8.5)/span/td + tdSTRING/td + tdcomma separated list of NEW,ESTABLISHED,RELATED,INVALID or NONE/td + /tr /table p brbr @@ -1168,6 +1193,11 @@ tdSTRING/td tdtext with max. 256 characters/td /tr + tr + tdstate span class=since(Since 0.8.5)/span/td + tdSTRING/td + tdcomma separated list of NEW,ESTABLISHED,RELATED,INVALID or NONE/td + /tr /table p brbr -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [patch 0/5] nwfilter: add a 'state' attribute to protocols
The following patch series introduces an attribute 'state' for iptables- supported protocols. This gives the user more control over the 'state match' of the underlying ip(6)tables implementation and allows to create filtering rules that are more efficient to evaluate. TCK test cases will be posted later. Each rule containing a state attribute with either one of the values NEW, ESTABLISHED, RELATED, INVALID or NONE, gets one iptables rule created for the direction of the rule. The keyword 'NONE' does the same, but doesn't generate a rule with the 'state match'. If no state attribute is used, a symmetric rule in the incoming and outgoing direction is generated (as was done previously). The patches do the following: - extend the parser and XML generator to parse and create XML with the state attribute - instantiate the state in case of ip(6)tables - extend the nwfilter.rng schema with the state attribute's possible values - add information about the new state attribute to the web docs - add a test case for the XML parser/generator to be run during 'make check' I valgrind'ed this patch and it looks all memory is freed appropriately. Regards, Stefan -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [patch 2/5] nwfilter: Instantiate state match in ip(6)tables rules
In this patch I am extending the rule instantiator to create the state
match according to the state attribute in the XML. Only one iptables
rule in the incoming or outgoing direction will be created for a rule
in direction 'in' or 'out' respectively. A rule in direction 'inout' does
get iptables rules in both directions.
Signed-off-by: Stefan Berger stef...@us.ibm.com
---
src/nwfilter/nwfilter_ebiptables_driver.c | 163 +-
1 file changed, 158 insertions(+), 5 deletions(-)
Index: libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
===
--- libvirt-acl.orig/src/nwfilter/nwfilter_ebiptables_driver.c
+++ libvirt-acl/src/nwfilter/nwfilter_ebiptables_driver.c
@@ -1129,7 +1129,7 @@ _iptablesCreateRuleInstance(int directio
const char *ifname,
virNWFilterHashTablePtr vars,
virNWFilterRuleInstPtr res,
-const char *match,
+const char *match, bool defMatch,
const char *accept_target,
bool isIPv6,
bool maySkipICMP)
@@ -1488,7 +1488,7 @@ _iptablesCreateRuleInstance(int directio
target = accept_target;
else {
target = DROP;
-skipMatch = true;
+skipMatch = defMatch;
}
if (match !skipMatch)
@@ -1548,6 +1548,149 @@ exit_no_error:
static int
+printStateMatchFlags(int32_t flags, char **bufptr)
+{
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+virNWFilterPrintStateMatchFlags(buf,
+-m state --state ,
+flags,
+false);
+if (virBufferError(buf)) {
+virBufferFreeAndReset(buf);
+virReportOOMError();
+return 1;
+}
+*bufptr = virBufferContentAndReset(buf);
+return 0;
+}
+
+static int
+iptablesCreateRuleInstanceStateCtrl(virNWFilterDefPtr nwfilter,
+virNWFilterRuleDefPtr rule,
+const char *ifname,
+virNWFilterHashTablePtr vars,
+virNWFilterRuleInstPtr res,
+bool isIPv6)
+{
+int rc;
+int directionIn = 0, directionOut = 0;
+char chainPrefix[2];
+bool maySkipICMP, inout = false;
+char *matchState = NULL;
+bool create;
+
+if ((rule-tt == VIR_NWFILTER_RULE_DIRECTION_IN) ||
+(rule-tt == VIR_NWFILTER_RULE_DIRECTION_INOUT)) {
+directionIn = 1;
+directionOut = inout = (rule-tt == VIR_NWFILTER_RULE_DIRECTION_INOUT);
+} else
+directionOut = 1;
+
+chainPrefix[0] = 'F';
+
+maySkipICMP = directionIn || inout;
+
+create = true;
+matchState = NULL;
+
+if (directionIn !inout) {
+if ((rule-flags IPTABLES_STATE_FLAGS))
+create = false;
+}
+
+if (create (rule-flags IPTABLES_STATE_FLAGS)) {
+if (printStateMatchFlags(rule-flags, matchState))
+return 1;
+}
+
+chainPrefix[1] = CHAINPREFIX_HOST_IN_TEMP;
+if (create) {
+rc = _iptablesCreateRuleInstance(directionIn,
+ chainPrefix,
+ nwfilter,
+ rule,
+ ifname,
+ vars,
+ res,
+ matchState, false,
+ RETURN,
+ isIPv6,
+ maySkipICMP);
+
+VIR_FREE(matchState);
+if (rc)
+return rc;
+}
+
+maySkipICMP = !directionIn || inout;
+create = true;
+
+if (!directionIn) {
+if ((rule-flags IPTABLES_STATE_FLAGS))
+create = false;
+}
+
+if (create (rule-flags IPTABLES_STATE_FLAGS)) {
+if (printStateMatchFlags(rule-flags, matchState))
+return 1;
+}
+
+chainPrefix[1] = CHAINPREFIX_HOST_OUT_TEMP;
+if (create) {
+rc = _iptablesCreateRuleInstance(!directionIn,
+ chainPrefix,
+ nwfilter,
+ rule,
+ ifname,
+ vars,
+ res,
+ matchState, false,
+ ACCEPT,
+ isIPv6,
+ maySkipICMP);
+
+VIR_FREE(matchState);
+
+if (rc)
+return rc;
+}
+
+maySkipICMP =
[libvirt] [patch 4/5] nwfilter: Extend schema to accept state attribute
Extend the nwfilter.rng schema to accept state attributes. Signed-off-by: Stefan Berger stef...@us.ibm.com --- docs/schemas/nwfilter.rng | 11 +++ 1 file changed, 11 insertions(+) Index: libvirt-acl/docs/schemas/nwfilter.rng === --- libvirt-acl.orig/docs/schemas/nwfilter.rng +++ libvirt-acl/docs/schemas/nwfilter.rng @@ -429,6 +429,11 @@ ref name=uint16range/ /attribute /optional + optional +attribute name=state + ref name=stateflags-type/ +/attribute + /optional /interleave /define @@ -860,4 +865,10 @@ define name='comment-type' data type=string/ /define + + define name='stateflags-type' +data type=string + param name=pattern((NEW|ESTABLISHED|RELATED|INVALID)(,(NEW|ESTABLISHED|RELATED|INVALID))*|NONE)/param +/data + /define /grammar -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [patch 3/5] nwfilter: Add test case for testing the state attribute
This patch adds a test case for testing the XML parser's and instantiator's support of the state attribute. The other test case tests existing capabilities. Both test cases will be used in TCK again. Signed-off-by: Stefan Berger stef...@us.ibm.com --- tests/nwfilterxml2xmlin/example-1.xml | 24 + tests/nwfilterxml2xmlin/example-2.xml | 37 + tests/nwfilterxml2xmlout/example-1.xml | 15 + tests/nwfilterxml2xmlout/example-2.xml | 21 ++ tests/nwfilterxml2xmltest.c|3 ++ 5 files changed, 100 insertions(+) Index: libvirt-acl/tests/nwfilterxml2xmlin/example-1.xml === --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlin/example-1.xml @@ -0,0 +1,24 @@ +filter name='testcase' + uuid01a992d2-f8c8-7c27-f69b-ab0a9d377379/uuid + + !-- allow incoming ssh connections -- + rule action='accept' direction='in' priority='100' +tcp dstportstart='22'/ + /rule + + !-- allow incoming ICMP (ping) packets -- + rule action='accept' direction='in' priority='200' +icmp/ + /rule + + !-- allow all outgoing traffic -- + rule action='accept' direction='in' priority='300' +all/ + /rule + + !-- drop all other traffic -- + rule action='drop' direction='inout' priority='1000' +all/ + /rule + +/filter Index: libvirt-acl/tests/nwfilterxml2xmlout/example-1.xml === --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlout/example-1.xml @@ -0,0 +1,15 @@ +filter name='testcase' chain='root' + uuid01a992d2-f8c8-7c27-f69b-ab0a9d377379/uuid + rule action='accept' direction='in' priority='100' +tcp dstportstart='22'/ + /rule + rule action='accept' direction='in' priority='200' +icmp/ + /rule + rule action='accept' direction='in' priority='300' +all/ + /rule + rule action='drop' direction='inout' priority='1000' +all/ + /rule +/filter Index: libvirt-acl/tests/nwfilterxml2xmltest.c === --- libvirt-acl.orig/tests/nwfilterxml2xmltest.c +++ libvirt-acl/tests/nwfilterxml2xmltest.c @@ -126,6 +126,9 @@ mymain(int argc, char **argv) DO_TEST(comment-test); +DO_TEST(example-1); +DO_TEST(example-2); + return (ret==0 ? EXIT_SUCCESS : EXIT_FAILURE); } Index: libvirt-acl/tests/nwfilterxml2xmlin/example-2.xml === --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlin/example-2.xml @@ -0,0 +1,37 @@ +filter name='testcase' + uuid01a992d2-f8c8-7c27-f69b-ab0a9d377379/uuid + + !-- VM outgoing: allow all established and related connections -- + rule action='accept' direction='out' priority='100' +all state='ESTABLISHED,RELATED' + comment='out: existing and related (ftp) connections'/ + /rule + + !-- VM incoming: allow all established connections -- + rule action='accept' direction='in' priority='100' +all state='ESTABLISHED' + comment='in: existing connections'/ + /rule + + !-- allow incoming ssh and ftp traffic -- + rule action='accept' direction='in' priority='200' +tcp dstportstart='21' dstportend='22' state='NEW' + comment='in: ftp and ssh'/ + /rule + + !-- allow incoming ICMP (ping) packets -- + rule action='accept' direction='in' priority='300' +icmp state='NEW' comment='in: icmp'/ + /rule + + !-- allow outgong DNS lookups -- + rule action='accept' direction='out' priority='300' +udp dstportstart='53' state='NEW' comment='out: DNS lookups'/ + /rule + + !-- drop all other traffic -- + rule action='drop' direction='inout' priority='1000' +all comment='inout: drop all non-accepted traffic'/ + /rule + +/filter Index: libvirt-acl/tests/nwfilterxml2xmlout/example-2.xml === --- /dev/null +++ libvirt-acl/tests/nwfilterxml2xmlout/example-2.xml @@ -0,0 +1,21 @@ +filter name='testcase' chain='root' + uuid01a992d2-f8c8-7c27-f69b-ab0a9d377379/uuid + rule action='accept' direction='out' priority='100' +all state='ESTABLISHED,RELATED' comment='out: existing and related (ftp) connections'/ + /rule + rule action='accept' direction='in' priority='100' +all state='ESTABLISHED' comment='in: existing connections'/ + /rule + rule action='accept' direction='in' priority='200' +tcp state='NEW' dstportstart='21' dstportend='22' comment='in: ftp and ssh'/ + /rule + rule action='accept' direction='in' priority='300' +icmp state='NEW' comment='in: icmp'/ + /rule + rule action='accept' direction='out' priority='300' +udp state='NEW' dstportstart='53' comment='out: DNS lookups'/ + /rule + rule action='drop' direction='inout' priority='1000' +all comment='inout: drop all non-accepted traffic'/ + /rule +/filter -- libvir-list mailing list libvir-list@redhat.com
