Hello Eric,
Am Mittwoch 27 Februar 2013, 02:00:07 schrieb Eric Blake:
> On 02/26/2013 05:42 AM, Philipp Hahn wrote:
> > Add format/@type entries to examples to show what the text is talking
> > about.
> >
> > Signed-off-by: Philipp Hahn
> > ---
> >
> > docs/storage.html.in |4
> > 1 file changed, 4 insertions(+)
> >
> > +++ b/docs/storage.html.in
> > @@ -185,6 +185,7 @@
> >
> > virtimages
> >
> >
> >
> >
> > +
> >
> >
>
> Question - is type="auto" safe, or does it risk the CVE where a raw
> image can be abused by a guest in a manner to make libvirt mis-detect
> the storage as some other type, and potentially causing libvirt to
> follow a backing chain outside of the guest's permitted reach?
Good question!
I just re-checked the three additions of which all
happen for storage pool, not storage volumes. So they are not accessible by
VMs.
> Depending on the answer, either this is safe to push as-is into 1.0.3,
> or we should revisit all mention of type="auto" to clarify the danger of
> relying on probing.
The "auto" are also the default from src/conf/storage_conf.c:
$ grep -n "defaultFormat = VIR_STORAGE_POOL_" src/conf/storage_conf.c
152:.defaultFormat = VIR_STORAGE_POOL_LOGICAL_LVM2,
167:.defaultFormat = VIR_STORAGE_POOL_FS_AUTO,
181:.defaultFormat = VIR_STORAGE_POOL_NETFS_AUTO,
239:.defaultFormat = VIR_STORAGE_POOL_DISK_UNKNOWN,
I chose "auto" because that looked like a safe default, before any admin
accidentally wipes his pools.
For the disk pool I chose "gpt" because "unknown" somehow looked strange and
"msdos" is limited to 2 TB, so the seconds recommendation looked best to me.
To me "auto" looks safe.
Sincerely
Philipp
--
Philipp Hahn Open Source Software Engineer h...@univention.de
Univention GmbHbe open. fon: +49 421 22 232- 0
Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99
http://www.univention.de/
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Wed, Feb 27, 2013 at 11:04 AM, Eric Blake wrote:
[...]
> You trimmed too much of your make line to tell us what was happening.
> It might also help to run 'make V=1' to get the full command line being
> attempted. I have to wonder if you have an unexpanded '$JAVA_HOME'
> injected somewhere into your configure output, where make is trying to
> compute $J (empty) followed by literal 'AVA_HOME'; maybe you should
> figure out what in your build setup is providing that bad variable value
> (correct would likely be '${JAVA_HOME}'). But I cannot reproduce your
> build failure, so it is something in your environment and not in libvirt
> itself that is causing you grief.
>
Eric, i was dizzy. Actually, i don't know why automake recognized
$JAVA_HOME as $AVA_HOME. However, i solved the problem like
following.
JVM_DIR=${shell echo ${JAVA_HOME}}
It works well for me. Thanks for your help ;-)
--
Thanks
Harry Wei
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
From: Li Zhang
When getting CPUs' information, it assumes that CPU indexes
are not contiguous. But for ppc64 platform, CPU indexes are not
contiguous because SMT is needed to be disabled, so CPU information
is not right on ppc64 and vpuinfo, vcpupin can't work corretly.
This patch is to remove the assumption to be compatible with ppc64.
Test:
4 vcpus are assigned to one VM and execute vcpuinfo command.
Without patch: There is only one vcpu informaion can be listed.
With patch: All vcpus' information can be listed correctly.
Signed-off-by: Li Zhang
---
src/qemu/qemu_monitor_json.c |7 ---
1 file changed, 7 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c
index 9991a0a..e130f8c 100644
--- a/src/qemu/qemu_monitor_json.c
+++ b/src/qemu/qemu_monitor_json.c
@@ -1230,13 +1230,6 @@ qemuMonitorJSONExtractCPUInfo(virJSONValuePtr reply,
goto cleanup;
}
-if (cpu != i) {
-virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unexpected cpu index %d expecting %d"),
- i, cpu);
-goto cleanup;
-}
-
threads[i] = thread;
}
--
1.7.10.1
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
https://bugzilla.redhat.com/show_bug.cgi?id=896092 mentions that
qemu 1.4 and earlier only accept a simple start-stop range for
the cpu=... argument of -numa. Libvirt would attempt to use
-numa cpu=1,3 for a disjoint range, which did not work as intended.
Upstream qemu will be adding a new syntax for disjoint cpu ranges
in 1.5; but the design for that syntax is still under discussion
at the time of this patch. So for libvirt 1.0.3, it is safest to
just reject attempts to build an invalid qemu command line; in the
future, we can add a capability bit and translate to the final
accepted design for selecting a disjoint cpu range in numa.
* src/qemu/qemu_command.c (qemuBuildNumaArgStr): Reject disjoint
ranges.
---
Also in response to:
https://www.redhat.com/archives/libvir-list/2013-February/msg01414.html
src/qemu/qemu_command.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 1c9bfc9..4f426e5 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4756,32 +4756,47 @@ qemuBuildNumaArgStr(const virDomainDefPtr def,
virCommandPtr cmd)
{
int i;
virBuffer buf = VIR_BUFFER_INITIALIZER;
-char *cpumask;
+char *cpumask = NULL;
+int ret = -1;
for (i = 0; i < def->cpu->ncells; i++) {
+VIR_FREE(cpumask);
virCommandAddArg(cmd, "-numa");
virBufferAsprintf(&buf, "node,nodeid=%d", def->cpu->cells[i].cellid);
virBufferAddLit(&buf, ",cpus=");
cpumask = virBitmapFormat(def->cpu->cells[i].cpumask);
if (cpumask) {
-virBufferAsprintf(&buf, "%s", cpumask);
-VIR_FREE(cpumask);
+/* Up through qemu 1.4, -numa does not accept a cpus
+ * argument any more complex than start-stop.
+ *
+ * XXX For qemu 1.5, the syntax has not yet been decided;
+ * but when it is, we need a capability bit and
+ * translation of our cpumask into the qemu syntax. */
+if (strchr(cpumask, ',')) {
+virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("disjoint NUMA cpu ranges are not supported "
+ "with this QEMU"));
+goto cleanup;
+}
+virBufferAdd(&buf, cpumask, -1);
}
def->cpu->cells[i].mem = VIR_DIV_UP(def->cpu->cells[i].mem,
1024) * 1024;
virBufferAsprintf(&buf, ",mem=%d", def->cpu->cells[i].mem / 1024);
-if (virBufferError(&buf))
-goto error;
+if (virBufferError(&buf)) {
+virReportOOMError();
+goto cleanup;
+}
virCommandAddArgBuffer(cmd, &buf);
}
-return 0;
+ret = 0;
-error:
+cleanup:
+VIR_FREE(cpumask);
virBufferFreeAndReset(&buf);
-virReportOOMError();
-return -1;
+return ret;
}
static int
--
1.8.1.2
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/26/2013 05:01 PM, Eugene Marcotte wrote:
> On Tue, 2013-02-26 at 16:42 -0700, Eric Blake wrote:
>> I think we are guaranteed that gcrypt.h exists if gnutls is properly
>> installed, even if it is a transitive dependency. Does this patch fix
>> it for you?
>
> Yup, at least when building without gnutls it works fine. When building
> with gnutls (e.g. after yum install gnutls-devel) it also works fine.
>
> Thanks!
With that review, I've gone ahead and pushed the patch.
>
>> diff --git i/src/libvirt.c w/src/libvirt.c
>> index 8a28e4a..0a21dea 100644
>> --- i/src/libvirt.c
>> +++ w/src/libvirt.c
>> @@ -2,7 +2,7 @@
>> * libvirt.c: Main interfaces for the libvirt library to handle
>> virtualization
>> * domains from a process running in domain 0
>> *
>> - * Copyright (C) 2005-2006, 2008-2012 Red Hat, Inc.
>> + * Copyright (C) 2005-2006, 2008-2013 Red Hat, Inc.
>> *
>> * This library is free software; you can redistribute it and/or
>> * modify it under the terms of the GNU Lesser General Public
>> @@ -31,7 +31,6 @@
>> #include
>> #include
>> #include
>> -#include
>>
>> #include
>> #include
>> @@ -56,6 +55,7 @@
>> #include "intprops.h"
>> #include "virconf.h"
>> #if WITH_GNUTLS
>> +# include
>> # include "rpc/virnettlscontext.h"
>> #endif
>> #include "vircommand.h"
>>
>>
>
>
>
>
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/26/2013 06:32 PM, harryxiyou wrote:
> Hi all,
>
> i got following errors.
>
> [...]
> cc1: error: AVA_HOME/include: No such file or directory [-Werror]
> cc1: error: AVA_HOME/include/linux: No such file or directory [-Werror]
> cc1: all warnings being treated as errors
> make[3]: *** [libvirt_driver_storage_impl_la-storage_driver.lo] Error 1
You trimmed too much of your make line to tell us what was happening.
It might also help to run 'make V=1' to get the full command line being
attempted. I have to wonder if you have an unexpanded '$JAVA_HOME'
injected somewhere into your configure output, where make is trying to
compute $J (empty) followed by literal 'AVA_HOME'; maybe you should
figure out what in your build setup is providing that bad variable value
(correct would likely be '${JAVA_HOME}'). But I cannot reproduce your
build failure, so it is something in your environment and not in libvirt
itself that is causing you grief.
--
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
On 02/26/2013 05:41 AM, Philipp Hahn wrote:
> Add qed for dirfs pool.
> Add ocfs2 for disk pool.
> Add lvm2 for disk and logical pool.
> Add cifs and glusterfs for netfs pool.
>
> Note: POOL_DISK_LVM2 can not be created by "parted mklabel", but is only
> returned from auto-detection on disk pools.
>
> Signed-off-by: Philipp Hahn
> ---
> docs/storage.html.in | 20 +++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
ACK, and I will push before 1.0.3.
--
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
On 02/26/2013 05:56 PM, Eric Blake wrote:
> On 02/26/2013 05:41 AM, Philipp Hahn wrote:
>> Add qed for dirfs pool.
>> Add ocfs2 for disk pool.
>> Add lvm2 for disk and logical pool.
>> Add cifs and glusterfs for netfs pool.
>>
>> Note: POOL_DISK_LVM2 can not be created by "parted mklabel", but is only
>> returned from auto-detection on disk pools.
>>
>> Signed-off-by: Philipp Hahn
>> ---
>> docs/storage.html.in | 20 +++-
>> 1 file changed, 19 insertions(+), 1 deletion(-)
>
> ACK, and I will push before 1.0.3.
Actually, this failed to build:
Validating storage.html
storage.html.tmp:535: element ul: validity error : Element ul content
does not follow the DTD, expecting (li)+, got (li li h3 p h2 p h3)
^
I made the appropriate tweak:
diff --git i/docs/storage.html.in w/docs/storage.html.in
index 92d7842..756d4b9 100644
--- i/docs/storage.html.in
+++ w/docs/storage.html.in
@@ -321,6 +321,7 @@
lvm2
+
Valid volume format types
--
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
On 02/26/2013 07:18 AM, John Ferlan wrote:
> Update the function prototypes to include a message about the client needing
> to free() returned name fields. Fix the all domains example flags values.
>
> v3->v2 diff:
> Separted this out to it's own patch.
s/Separted/Separated/
then again, the information about v3->v2 diff belongs after a --- line,
so that 'git am' won't put it into git history (if someone else is
applying the patch). If you are pushing the patch yourself, then be
sure to edit the commit message to drop version history at that time
(it's useful on list, but doesn't need to be part of the permanent logs).
> ---
> src/libvirt.c | 21 ++---
> 1 file changed, 10 insertions(+), 11 deletions(-)
ACK. Let's get this into 1.0.3.
--
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
On 02/26/2013 07:20 AM, John Ferlan wrote:
I would have put '---' here, since...
> v3->v2 difference: Reduced the amount of change to a few comments and
> adjusting the (NULL == xxx) and (-1 == xxx) checks
>
> Since these are just documentation changes - once ACK'd is it OK to push
> now or should I wait for after the freeze?
>
> Tks,
...this information isn't useful in the final git log, but makes sense
in reviewing. This patch is safe for 1.0.3 (as you point out, it is a
doc improvement, and can't cause any bugs in libvirtd)
>
> John
> ---
> examples/hellolibvirt/hellolibvirt.c | 26 ++
> 1 file changed, 14 insertions(+), 12 deletions(-)
>
> @@ -93,7 +94,7 @@ showDomains(virConnectPtr conn)
> char **nameList = NULL;
>
> numActiveDomains = virConnectNumOfDomains(conn);
> -if (-1 == numActiveDomains) {
> +if (numActiveDomains == -1) {
> ret = 1;
> printf("Failed to get number of active domains\n");
> showError(conn);
virConnectNumOfDomains is inherently racy. Wouldn't it be better to
just drop this section of our example?
> @@ -101,7 +102,7 @@ showDomains(virConnectPtr conn)
> }
>
> numInactiveDomains = virConnectNumOfDefinedDomains(conn);
> -if (-1 == numInactiveDomains) {
> +if (numInactiveDomains == -1) {
> ret = 1;
> printf("Failed to get number of inactive domains\n");
> showError(conn);
Same question.
> @@ -113,17 +114,20 @@ showDomains(virConnectPtr conn)
>
> nameList = malloc(sizeof(*nameList) * numInactiveDomains);
>
> -if (NULL == nameList) {
> +if (!nameList) {
> ret = 1;
> printf("Could not allocate memory for list of inactive domains\n");
> goto out;
> }
>
> +/* The virConnectListDomains() will return a list of the active domains.
> + * Alternatively the virConnectListAllDomains() API would return a list
> + * of both active and inactive domains */
> numNames = virConnectListDefinedDomains(conn,
> nameList,
> numInactiveDomains);
I think it would be better to update the example to JUST use
virConnectListAllDomains(), and completely avoid
virConnectListDefinedDomains.
>
> -if (-1 == numNames) {
> +if (numNames == -1) {
> ret = 1;
> printf("Could not get list of defined domains from hypervisor\n");
> showError(conn);
> @@ -136,9 +140,7 @@ showDomains(virConnectPtr conn)
>
> for (i = 0 ; i < numNames ; i++) {
> printf(" %s\n", *(nameList + i));
> -/* The API documentation doesn't say so, but the names
> - * returned by virConnectListDefinedDomains are strdup'd and
> - * must be freed here. */
> +/* must free the returned named per the API documentation */
> free(*(nameList + i));
Pre-existing, but '*(nameList + i)' looks odd; 'nameList[i]' looks nicer.
--
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
On 02/26/2013 05:42 AM, Philipp Hahn wrote:
> Add format/@type entries to examples to show what the text is talking
> about.
>
> Signed-off-by: Philipp Hahn
> ---
> docs/storage.html.in |4
> 1 file changed, 4 insertions(+)
> +++ b/docs/storage.html.in
> @@ -185,6 +185,7 @@
> virtimages
>
>
> +
>
Question - is type="auto" safe, or does it risk the CVE where a raw
image can be abused by a guest in a manner to make libvirt mis-detect
the storage as some other type, and potentially causing libvirt to
follow a backing chain outside of the guest's permitted reach?
Depending on the answer, either this is safe to push as-is into 1.0.3,
or we should revisit all mention of type="auto" to clarify the danger of
relying on probing.
--
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
On 02/26/2013 09:08 AM, Michal Privoznik wrote:
> Currently, if we label a file to match qemu process DAC label, we
> do not store the original owner anywhere. So when relabeling
> back, the only option we have is to relabel to root:root
> which is obviously wrong.
>
> However, bare remembering is not enough. We need to keep track of
> how many times we labeled a file so only the last restore
> chown()-s file back to the original owner.
Definitely important for a read-only file shared by more than one domain.
>
> In order to not pollute domain XML, this info is kept in driver's
> private data in a hash table with path being key and pair
> being value.
Makes sense.
Have you looked at what it would take to use ACLs to grant access to
qemu without having to do a full-blown chown? That would also need to
use the hash table to undo the ACL at the end of the day, and we would
need to fall back to chown() on file systems where ACL doesn't work, but
it certainly sounds like that would be sharing some of the work in this
patch.
> ---
> src/security/security_dac.c| 351
> ++---
> src/security/security_driver.h | 3 +
> 2 files changed, 296 insertions(+), 58 deletions(-)
Couple of questions below:
>
> diff --git a/src/security/security_dac.c b/src/security/security_dac.c
> index 0b274b7..4b8f0a2 100644
> --- a/src/security/security_dac.c
> +++ b/src/security/security_dac.c
> @@ -42,8 +42,121 @@ struct _virSecurityDACData {
> uid_t user;
> gid_t group;
> bool dynamicOwnership;
> +virHashTablePtr oldOwners; /* to hold pairs */
> };
>
> +struct _virOldLabel {
> +char *owner;
Are you really planning on storing a string uid:gid? Wouldn't it be
simpler to store a uid_t and gid_t as read from struct stat, as long as
the data is only in memory? And when storing the data to disk in XML to
survive libvirtd restarts, it seems like storing two attributes
user='nnn' group='nnn' is nicer than storing one attribute
owner='+nnn:+nnn' that requires further parsing back into user and group.
> +int refCount;
> +};
> +
> +static void virOldLabelFree(virOldLabelPtr label)
Line break after void.
> +/**
> + * virSecurityDACRememberLabel:
> + * @priv: private DAC driver data
> + * @path: path which is about to be relabelled
> + *
> + * Store the original DAC label of @path.
> + * Returns: number of references of @path, or -1 on error
> + */
> +static int
> +virSecurityDACRememberLabel(virSecurityDACDataPtr priv,
> +const char *path)
> +{
> +struct stat sb;
> +virOldLabelPtr oldLabel = NULL;
> +char *user = NULL, *group = NULL;
> +
> +oldLabel = virHashLookup(priv->oldOwners, path);
> +
> +if (oldLabel) {
> +/* just increment ref counter */
> +oldLabel->refCount++;
Is this threadsafe, or do we need to use virAtomic?
> +goto cleanup;
> +}
> +
> +if (stat(path, &sb) < 0) {
> +virReportSystemError(errno, _("Unable to stat %s"), path);
> +goto cleanup;
> +}
> +
> +user = virGetUserName(sb.st_uid);
> +group = virGetGroupName(sb.st_gid);
> +if ((!user && (virAsprintf(&user, "+%ld", (long) sb.st_uid) < 0)) ||
> +(!group && (virAsprintf(&group, "+%ld", (long) sb.st_gid) < 0)) ||
Given Philipp's recent series, it's probably better to mix %u and
(unsigned), rather than %ld and (long) (that is, we've already asserted
that uid_t fits within an int, and that unsigned is more likely than
signed to represent all users except for the sentinel of -1, and -1 is
not going to appear as stat() results).
Again, if your struct has a uid_t and gid_t instead of a char*, then you
don't have to convert to string here.
> +(VIR_ALLOC(oldLabel) < 0) ||
> +(virAsprintf(&oldLabel->owner, "%s:%s", user, group) < 0)) {
Worse, you are printing into two temporaries only to create yet another
malloc'd string. If you do keep a char*, why not just:
virAsprintf(&oldLabel->owner, "+%u:+%u", (unsigned) sb.st_uid,
(unsigned) sb.st_gid);
> +
> +static int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr);
Any reason you can't topologically sort that function to be first, and
avoid a forward declaration?
> +
> +/**
> + * virSecurityDACGetRememberedLabel:
> + * @priv: private DAC driver data
> + * @path: path which we want to restore label on
> + * @user: original owner of @path
> + * @group: original owner of @path
> + *
> + * Decrements reference counter on @path. If this was the last
> + * reference, we need to restore the original label, in which
> + * case @user and @group are updated.
> + * Returns: the number of references of @path
> + * 0 if we need to restore the label
> + * -1 on error
> + */
> +static int
> +virSecurityDACGetRememberedLabel(virSecurityDACDataPtr priv,
> + const char *path,
> + uid_t *user,
> +
On 2013年02月27日 07:28, Eric Blake wrote:
On 02/25/2013 10:22 PM, Li Zhang wrote:
Hi all,
In this function qemuMonitorJSONExtractCPUInfo(),
It assumes that cpu index is always contiguous.
If possible, can this assumption be removed?
Sure - patches welcome. It would be best if you can submit the patch,
since you are the one being hit by the issue, and can test if your patch
works. Asking anyone else to do the work when they can't reproduce the
problem is going to be slower.
Sure, I will send out patch.
Thanks a lot. :)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Hi all,
When i compiled libvirt like following
1. git clone git://libvirt.org/libvirt.git；cd libvirt
2. git reset --hard v1.0.1
3. wget
http://cloudxy.googlecode.com/svn/branches/hlfs/person/harry/hlfs/patches/hlfs_driver_for_libvirt_network_disk.patch
4. wget
http://cloudxy.googlecode.com/svn/branches/hlfs/person/harry/hlfs/patches/hlfs_driver_for_libvirt_add_classpath.patch
5. git apply hlfs_driver_for_libvirt_network_disk.patch
6. git apply hlfs_driver_for_libvirt_add_classpath.patch
7. ./autogen.sh
8. ./configure
9. make
i got following errors.
[...]
cc1: error: AVA_HOME/include: No such file or directory [-Werror]
cc1: error: AVA_HOME/include/linux: No such file or directory [-Werror]
cc1: all warnings being treated as errors
make[3]: *** [libvirt_driver_storage_impl_la-storage_driver.lo] Error 1
make[3]: Leaving directory `/home/jiawei/workshop1/libvirt/src'
make[2]: *** [all] Error 2
make[2]: Leaving directory `/home/jiawei/workshop1/libvirt/src'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/jiawei/workshop1/libvirt'
make: *** [all] Error 2
It seems that the errors have no relationship with HLFS(the patches i patched)
but AVA_HOME.
Could anyone give me some suggestions? Thanks in advance ;-)
--
Thanks
Harry Wei
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 27/02/13 01:01, TJ wrote:
> I've added a new variable to 'configure.a' (DHCPRELAY) but found that
> 'autogen.sh' doesn't call
> 'autoheader' to remake 'config.h.in' so it contains the definition of
> DHCPRELAY for the compiler.
False alarm - I made a typo in the most critical spot in 'configure.ac', based
on an earlier version of the patches, which used a different pre-processor
variable name.
"./autogen.sh" now works as expected.
Tiredness Kills... build scripts :)
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
I'm currently adding support for running a DHCP relay daemon instead of using
dnsmasq for DHCP.
Everything is looking good but I've stumbled on one issue.
I've added a new variable to 'configure.a' (DHCPRELAY) but found that
'autogen.sh' doesn't call
'autoheader' to remake 'config.h.in' so it contains the definition of DHCPRELAY
for the compiler.
Reading 'autogen.sh' and 'bootstrap' I can see convoluted functionality but
can't figure out
what the recommended best practice would be in terms of generating the new
'config.h.in'.
If I call 'autoheader' manually the definition is added correctly.
Should I just do that and then add the changed 'config.h.in' to the commit that
introduces the
new definition?
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, 2013-02-26 at 16:42 -0700, Eric Blake wrote:
> Ah, I see now - the file in question is src/libvirt.c - the typo of
> libirt.c threw me off.
Sorry about that.
> I think we are guaranteed that gcrypt.h exists if gnutls is properly
> installed, even if it is a transitive dependency. Does this patch fix
> it for you?
Yup, at least when building without gnutls it works fine. When building
with gnutls (e.g. after yum install gnutls-devel) it also works fine.
Thanks!
> diff --git i/src/libvirt.c w/src/libvirt.c
> index 8a28e4a..0a21dea 100644
> --- i/src/libvirt.c
> +++ w/src/libvirt.c
> @@ -2,7 +2,7 @@
> * libvirt.c: Main interfaces for the libvirt library to handle
> virtualization
> * domains from a process running in domain 0
> *
> - * Copyright (C) 2005-2006, 2008-2012 Red Hat, Inc.
> + * Copyright (C) 2005-2006, 2008-2013 Red Hat, Inc.
> *
> * This library is free software; you can redistribute it and/or
> * modify it under the terms of the GNU Lesser General Public
> @@ -31,7 +31,6 @@
> #include
> #include
> #include
> -#include
>
> #include
> #include
> @@ -56,6 +55,7 @@
> #include "intprops.h"
> #include "virconf.h"
> #if WITH_GNUTLS
> +# include
> # include "rpc/virnettlscontext.h"
> #endif
> #include "vircommand.h"
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/25/2013 06:14 PM, Eugene Marcotte wrote:
> On Mon, 2013-02-25 at 17:42 -0700, Eric Blake wrote:
>> On 02/25/2013 05:38 PM, Eugene Marcotte wrote:
>>> Hi,
>>>
>>> I'm building libvirt for the first time and figuring out what libraries
>>> are required to get the simplest possible thing working.
>>>
>>> Is there a hard requirement on gcrypt? I got libvirt past the configure
>>> script but the build failed on gcrypt.h. It looks like the include for
>>> it could be moved inside the #if WITH_GNUTLS section of libirt.c, if I
>>> understand what configure.ac is trying to set up.
Ah, I see now - the file in question is src/libvirt.c - the typo of
libirt.c threw me off.
>>
>> Can you show the exact compiler error message you received? You are
>> correct that this header is non-standard, and should be properly ifdef'd
>> before we try to use it.
>>
>
> make[3]: Entering directory `/home/emarcotte/src/libvirt/src'
> CC libvirt_driver_la-libvirt.lo
> libvirt.c:34:20: fatal error: gcrypt.h: No such file or directory
> compilation terminated.
Other than the broken #include, it looks like the rest of the file
protects all use of *gcr* and *GCR* under the WITH_GNUTLS conditional;
and no other file seemed to be impacted.
>
> Of course, installing gcrypt-devel fixes it, but I would have expected
> configure to fail. I can try to add an explicit check for gcrypt.h if
> it's required.
I think we are guaranteed that gcrypt.h exists if gnutls is properly
installed, even if it is a transitive dependency. Does this patch fix
it for you?
diff --git i/src/libvirt.c w/src/libvirt.c
index 8a28e4a..0a21dea 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -2,7 +2,7 @@
* libvirt.c: Main interfaces for the libvirt library to handle
virtualization
* domains from a process running in domain 0
*
- * Copyright (C) 2005-2006, 2008-2012 Red Hat, Inc.
+ * Copyright (C) 2005-2006, 2008-2013 Red Hat, Inc.
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
@@ -31,7 +31,6 @@
#include
#include
#include
-#include
#include
#include
@@ -56,6 +55,7 @@
#include "intprops.h"
#include "virconf.h"
#if WITH_GNUTLS
+# include
# include "rpc/virnettlscontext.h"
#endif
#include "vircommand.h"
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/26/2013 03:16 AM, Viktor Mihajlovski wrote:
> Can we please revert the following commits before rc2;
>
> 24aa7f8d11054b7b2e643cf3cd5c80a199764af0 (S390: Documentation for CCW
> address type)
> 0bbbd42c30543d8251536c2fa11166834c886ada (S390: domain_conf support for
> CCW)
>
> this is for two reasons:
> 1. They don't make sense without the rest of the series which is still
> pending review (and probably needs a rebase meanwhile) and the result
> already shows up on libvirt.org
> 2. I received feedback from someone more platform-savvy than I am, that
> one of the XML attribute names (schid) is misleading
Yes, avoiding the trap of releasing XML early, only to force ourselves
into supporting that design forever, seems like a reasonable reason to
revert while waiting for the more complete design to get a thorough review.
>
> Reverting gives me the opportunity to provide a new series on top of
> 1.0.3 which can be hopefully applied in time for the next release.
Thanks for bringing up the question; I've reverted those patches as
requested. It also lends a bit more weight to the question about DHCP
, which is another XML addition where we are not sure if the
final design is ready for 1.0.3.
--
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
On 02/25/2013 10:22 PM, Li Zhang wrote:
> Hi all,
>
> In this function qemuMonitorJSONExtractCPUInfo(),
>
> It assumes that cpu index is always contiguous.
>
> If possible, can this assumption be removed?
Sure - patches welcome. It would be best if you can submit the patch,
since you are the one being hit by the issue, and can test if your patch
works. Asking anyone else to do the work when they can't reproduce the
problem is going to be slower.
--
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
On 02/26/2013 04:02 AM, Michal Privoznik wrote:
> Currently, qemuDomainShutdownFlags() chooses the agent method of
> shutdown whenever the agent is configured. However, this
> assumption is not enough as the guest agent may be unresponsive
> at the moment. So unless guest agent method has been explicitly
> requested, we should fall back to the ACPI method.
> ---
>
> diff to v1:
> - Rework some conditions as Eric suggested in v1
>
> src/qemu/qemu_driver.c | 38 ++
> 1 file changed, 22 insertions(+), 16 deletions(-)
ACK.
--
Eric Blake eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Paolo Bonzini writes:
> Il 26/02/2013 20:35, Anthony Liguori ha scritto:
>>
>> See also discussion on multi-valued keys in command line option
>> arguments and config files in v1 thread. Hopefully we can reach a
>> conclusion soon, and then we'll see whether this patch is what we want.
>>> >
>>> > Yeah, let's drop this patch by now. I am starting to be convinced that
>>> > "cpus=A,cpus=B,cpus=C" is the best approach. It is not pretty, but at
>>> > least it uses generic parser code instead of yet another ad-hoc
>>> > parser.
>> No, we cannot rely on this behavior. We had to do it to support
>> backwards compat with netdev but it should not be used anywhere else.
>
> We chose a config format (git's) because it was a good match for
> QemuOpts, and multiple-valued operations are well supported by that
> config format; see git-config(1). There is no reason to consider it a
> backwards-compatibility hack.
Seconded.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[Bcc problems] Ah, indeed, I never got some of those mails, just saw my
original patch merged last Sunday and already wondered why nobody
commented :)
For brevity, I'll summarize the points we've been discussing. Forgive me
if I oversee something important.
- We're thinking about importing DHCP options as "first class citizens"
into our XML language.
- Issue: There are no official names in any RFC.
- Possible solution: use what dnsmasq offers ("dnsmasq --help dhcp").
- Issue here: We do not want to depend on dnsmasq peculiariaties
too much. Consider the situation where we choose a different
DHCP server and that one uses different names. We'd have to
convert from nameA to nameB.
- Issue here: The selection made by dnsmasq could be called
somewhat subjective. There is a great possibility that users
need additional options, be it more rarely used,
hardware-specific options for some access point, or the custom
option range (224-252).
- Possible solution: go with names for everything dnsmasq also
has a name for and allow numbers for other DHCP options.
- Issue here: If we take this route, we need to keep the names
synchronous to dnsmasq. Eg. if dnsmasq learns a new option, we
should add it to our XML schema as well for consistency reasons.
- Possible solution: invent own names for further options.
- Issue here: Not much utility and names won't be too intuitive.
- Issue: whatever names we take, it hurts much more if in retrospect
we chose the "wrong" ones than it does to allow . So while
allowing numbers at first sight looks like a possible trapdoor,
I guess getting the names right is harder because it's sort of a
decision "for eternity".
- Issue: If we learn names, we might want to validate values just as
well.
- Issue: One could implement different depths of option validation.
For example, according to RFC 4578 DHCP option 94 (Client network
interface identifier) has a three octets-sized value and currently
the first one can only be 1. Would we enforce this or just enforce
three integer octets?
- Issue: As a variation, some clients might not be 100%
RFC-conformant and expect certain option values that we forbid
because _we_ try to conform 100% to the appropriate RFC.
- Issue: We would have to select for WHICH options we'd do
validation. The underlying DHCP server, be it dnsmasq or something
else in the future, is possibly not as restrictive as we are. The
alternative approach "be as smart as dnsmasq is" will be hard to
maintain.
- Issue: Validation can't be done for the "private use" DHCP
options.
- Possible solution: Just allow anything for THESE.
Unrelated to this discussion there was the idea to add a "force="
attribute. I don't think there's much argument about it, it's a good idea.
Am 26.02.2013 19:05, schrieb Laine Stump:
On the other hand, I don't want to over-engineer this problem so much
that we never push *anything*.
Yes, unless I missed something I think the bottom line of my summary
above is that there would have to be a really good argument _against_
"number=xxx". Implementing "name=yyy" does not automatically mean that
"number=xxx" is a bad thing to do. Most of all, it's a very pragmatic
solution whereas "name=xxx" most probably requires some design decisions.
Without even arriving at a decision about this, I'm now thinking that
maybe we should revert the earlier patch until after the
release, and re-push it after the named-option support is done
(potentially with some changes).
Any other opinions?
I'm used to having to compile it myself anyway, so that'd be fine with
me if we want
to give it more thought.
On the other hand, I don't see that we can do WITHOUT "number=..."
unless we want to implement only a restricted subset of DHCP options
known by name.
+ ipdef->options[r].number,
+ ipdef->options[r].value);
We've done no sanity checking on the contents of value. Is there any
danger of a rogue metacharacter in dnsmasq.conf causing problems? I seem
to recall asking that question before and being told "no", and I'm going
to continue assuming that for now, but I still think it would be good
practice to do some basic validation of the value if we can determine an
all-encompassing valid character set that is something smaller than
"everything" :-)
dnsmasq (or any other DHCP server we support in the future) could use a
"test config" mode that simply returns success or failure and that we
could use when reading in XML fragments. That way, we wouldn't have to
duplicate dnsmasq's work.
@@ -959,6 +968,7 @@ networkDnsmasqConfContents(virNetworkObjPtr network,
virBufferAsprintf(&configbuf, "addn-hosts=%s\n",
dctx->addnhostsfile->path);
+
Thi
Il 26/02/2013 20:35, Anthony Liguori ha scritto:
>>> >>
>>> >> See also discussion on multi-valued keys in command line option
>>> >> arguments and config files in v1 thread. Hopefully we can reach a
>>> >> conclusion soon, and then we'll see whether this patch is what we want.
>> >
>> > Yeah, let's drop this patch by now. I am starting to be convinced that
>> > "cpus=A,cpus=B,cpus=C" is the best approach. It is not pretty, but at
>> > least it uses generic parser code instead of yet another ad-hoc
>> > parser.
> No, we cannot rely on this behavior. We had to do it to support
> backwards compat with netdev but it should not be used anywhere else.
We chose a config format (git's) because it was a good match for
QemuOpts, and multiple-valued operations are well supported by that
config format; see git-config(1). There is no reason to consider it a
backwards-compatibility hack.
Paolo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Feb 26, 2013 at 01:35:14PM -0600, Anthony Liguori wrote:
> Eduardo Habkost writes:
>
> > On Tue, Feb 26, 2013 at 11:50:08AM +0100, Markus Armbruster wrote:
> >> Eduardo Habkost writes:
> >>
> >> > This allows ":" to be used a separator between each CPU range, so the
> >> > command-line may look like:
> >> >
> >> > -numa node,cpus=A-B:C-D
> >> >
> >> > Note that the following format, currently used by libvirt:
> >> >
> >> > -numa nodes,cpus=A-B,C-D
> >> >
> >> > will _not_ work, as "," is the option separator for the command-line
> >> > option parser, and it would require changing the -numa option parsing
> >> > code to handle "cpus" as a special case.
> >> >
> >> > Signed-off-by: Eduardo Habkost
> >> > ---
> >> > Changes v2:
> >> > - Use ":" as separator
> >> > - Document the new format
> >>
> >> See also discussion on multi-valued keys in command line option
> >> arguments and config files in v1 thread. Hopefully we can reach a
> >> conclusion soon, and then we'll see whether this patch is what we want.
> >
> > Yeah, let's drop this patch by now. I am starting to be convinced that
> > "cpus=A,cpus=B,cpus=C" is the best approach. It is not pretty, but at
> > least it uses generic parser code instead of yet another ad-hoc
> > parser.
>
> No, we cannot rely on this behavior. We had to do it to support
> backwards compat with netdev but it should not be used anywhere else.
Why? What should be the proper and generic way to represent and parse
lists, then?
There are many arguments being exposed at the v1 thread. See
Message-ID: <512cc6d6.4060...@redhat.com>.
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Eduardo Habkost writes:
> On Tue, Feb 26, 2013 at 11:50:08AM +0100, Markus Armbruster wrote:
>> Eduardo Habkost writes:
>>
>> > This allows ":" to be used a separator between each CPU range, so the
>> > command-line may look like:
>> >
>> > -numa node,cpus=A-B:C-D
>> >
>> > Note that the following format, currently used by libvirt:
>> >
>> > -numa nodes,cpus=A-B,C-D
>> >
>> > will _not_ work, as "," is the option separator for the command-line
>> > option parser, and it would require changing the -numa option parsing
>> > code to handle "cpus" as a special case.
>> >
>> > Signed-off-by: Eduardo Habkost
>> > ---
>> > Changes v2:
>> > - Use ":" as separator
>> > - Document the new format
>>
>> See also discussion on multi-valued keys in command line option
>> arguments and config files in v1 thread. Hopefully we can reach a
>> conclusion soon, and then we'll see whether this patch is what we want.
>
> Yeah, let's drop this patch by now. I am starting to be convinced that
> "cpus=A,cpus=B,cpus=C" is the best approach. It is not pretty, but at
> least it uses generic parser code instead of yet another ad-hoc
> parser.
No, we cannot rely on this behavior. We had to do it to support
backwards compat with netdev but it should not be used anywhere else.
Regards,
Anthony Liguori
>
> --
> Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/26/2013 01:21 PM, Eric Blake wrote:
> [adding Pieter, in case he didn't see the reply.]
I'm starting to think there's a bug in Thunderbird - I pasted Pieter's
address into a line that had "Bcc:" on it, then changed the Bcc to Cc; I
specifically remember doing this after my recent experience with an
attempted Cc to libvir-list ending up as a Bcc.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[adding Pieter, in case he didn't see the reply.]
On 02/26/2013 11:05 AM, Laine Stump wrote:
> My original response to the patch and to Michal's review is at the end
> of this message for historical purposes. I had written it up, made some
> small changes to the patch to address problems I found, and pushed it,
> but somehow managed to never hit "Send" on the email :-(
>
> In the meantime, I read Eric's review of my followup patch that added
> support for a "force" attribute:
>
>https://www.redhat.com/archives/libvir-list/2013-February/msg01413.html
>
> Eric's self-debate here got me thinking more seriously about the future
> implications of this original "numeric-only" patch, and of
> something that has been becoming more apparent to me as I've been taking
> a whack at implementing support for named options on top of it.
>
> In particular, the "value" of a dhcp option can be one of several types,
> and in some cases it can be a repeated list (especially of IP addresses,
> but there are a few that are lists of numbers). As it stands, we treat
> "value" as an opaque string, and just pass it through to dnsmasq as-is
> (except for checking for embedded spaces and \). For example, this:
>
>
>
> results in the following line in dnsmasq.conf:
>
> dhcp-option=6,1.2.3.4,5.6.7.8
>
> That works, but if we ever decide we want to do a better job of
> validating the value (rather than just relying on dnsmasq), then we
> arrive at a situation where the data that has been parsed out of the XML
> must be further subdivided and parsed to get at the individual values in
> the list. I can recall at least twice in recent memory that I dinged a
> patch during review for exactly this reason.
>
> On the other hand, I don't want to over-engineer this problem so much
> that we never push *anything*.
>
> Without even arriving at a decision about this, I'm now thinking that
> maybe we should revert the earlier patch until after the
> release, and re-push it after the named-option support is done
> (potentially with some changes).
I also like the idea of reverting this change so that 1.0.3 doesn't
leave us stuck with a half-baked design that we will later regret
because we have to do back-compat support alongside a more friendly
design with option names. Having a bit more time on our side to get the
interface right in 1.0.4 feels like it will be beneficial in the long
run. Pieter, can you chime in within the next 24 hours? If not, I'm
okay if Laine does the revert before DV cuts rc2.
>
> Any other opinions?
>
>
>
> ===
> On 02/22/2013 04:37 AM, Michal Privoznik wrote:
>> On 21.02.2013 23:40, Pieter Hollants wrote:
>>> This patch adds support for a new -Tag in the block of
>>> network configs,
>>> based on a subset of the fifth proposal by Laine Stump in the mailing list
>>> discussion at
>>> https://www.redhat.com/archives/libvir-list/2012-November/msg01054.html.
>>> Any such defined
>>> option will result in a dhcp-option=,"" statement in the
>>> generated dnsmasq
>>> configuration file.
>>>
>>> Currently, DHCP options can be specified by number only and there is no
>>> whitelisting or
>>> blacklisting of option numbers, which should probably be added.
>>>
>>> Signed-off-by: Pieter Hollants
>>> ---
>>> AUTHORS.in|1 +
>>> docs/schemas/network.rng |6 +++
>>> docs/schemas/networkcommon.rng|6 +++
>>> src/conf/network_conf.c | 54
>>> +
>>> src/conf/network_conf.h | 10
>>> src/network/bridge_driver.c | 10
>>> tests/networkxml2xmlin/netboot-network.xml|1 +
>>> tests/networkxml2xmlin/netboot-proxy-network.xml |1 +
>>> tests/networkxml2xmlout/netboot-network.xml |1 +
>>> tests/networkxml2xmlout/netboot-proxy-network.xml |1 +
>>> 10 Dateien geändert, 91 Zeilen hinzugefügt(+)
>>>
>>> diff --git a/AUTHORS.in b/AUTHORS.in
>>> index 39fe68d..074185e 100644
>>> --- a/AUTHORS.in
>>> +++ b/AUTHORS.in
>>> @@ -74,6 +74,7 @@ Michel Ponceau
>>> Nobuhiro Itou
>>> Pete Vetere
>>> Philippe Berthault
>>> +Pieter Hollants
>>> Saori Fukuta
>>> Shigeki Sakamoto
>>> Shuveb Hussain
>> This is not necessary. AUTHORS file is now completely generated since
>> v0.10.2-207-g7b21981.
>>
>>> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
>>> index 09d7c73..a2ce1c9 100644
>>> --- a/docs/schemas/network.rng
>>> +++ b/docs/schemas/network.rng
>>> @@ -293,6 +293,12 @@
>>>
>>>
>>>
>>> +
>>> +
>>> +>> name="unsignedByte"/>
>>> +
>
> Actually it appears that it's legal to send some options with no value
> at all; this is even mentioned in dnsmasq documen
My original response to the patch and to Michal's review is at the end
of this message for historical purposes. I had written it up, made some
small changes to the patch to address problems I found, and pushed it,
but somehow managed to never hit "Send" on the email :-(
In the meantime, I read Eric's review of my followup patch that added
support for a "force" attribute:
https://www.redhat.com/archives/libvir-list/2013-February/msg01413.html
Eric's self-debate here got me thinking more seriously about the future
implications of this original "numeric-only" patch, and of
something that has been becoming more apparent to me as I've been taking
a whack at implementing support for named options on top of it.
In particular, the "value" of a dhcp option can be one of several types,
and in some cases it can be a repeated list (especially of IP addresses,
but there are a few that are lists of numbers). As it stands, we treat
"value" as an opaque string, and just pass it through to dnsmasq as-is
(except for checking for embedded spaces and \). For example, this:
results in the following line in dnsmasq.conf:
dhcp-option=6,1.2.3.4,5.6.7.8
That works, but if we ever decide we want to do a better job of
validating the value (rather than just relying on dnsmasq), then we
arrive at a situation where the data that has been parsed out of the XML
must be further subdivided and parsed to get at the individual values in
the list. I can recall at least twice in recent memory that I dinged a
patch during review for exactly this reason.
On the other hand, I don't want to over-engineer this problem so much
that we never push *anything*.
Without even arriving at a decision about this, I'm now thinking that
maybe we should revert the earlier patch until after the
release, and re-push it after the named-option support is done
(potentially with some changes).
Any other opinions?
===
On 02/22/2013 04:37 AM, Michal Privoznik wrote:
> On 21.02.2013 23:40, Pieter Hollants wrote:
>> This patch adds support for a new -Tag in the block of
>> network configs,
>> based on a subset of the fifth proposal by Laine Stump in the mailing list
>> discussion at
>> https://www.redhat.com/archives/libvir-list/2012-November/msg01054.html. Any
>> such defined
>> option will result in a dhcp-option=,"" statement in the
>> generated dnsmasq
>> configuration file.
>>
>> Currently, DHCP options can be specified by number only and there is no
>> whitelisting or
>> blacklisting of option numbers, which should probably be added.
>>
>> Signed-off-by: Pieter Hollants
>> ---
>> AUTHORS.in|1 +
>> docs/schemas/network.rng |6 +++
>> docs/schemas/networkcommon.rng|6 +++
>> src/conf/network_conf.c | 54
>> +
>> src/conf/network_conf.h | 10
>> src/network/bridge_driver.c | 10
>> tests/networkxml2xmlin/netboot-network.xml|1 +
>> tests/networkxml2xmlin/netboot-proxy-network.xml |1 +
>> tests/networkxml2xmlout/netboot-network.xml |1 +
>> tests/networkxml2xmlout/netboot-proxy-network.xml |1 +
>> 10 Dateien geändert, 91 Zeilen hinzugefügt(+)
>>
>> diff --git a/AUTHORS.in b/AUTHORS.in
>> index 39fe68d..074185e 100644
>> --- a/AUTHORS.in
>> +++ b/AUTHORS.in
>> @@ -74,6 +74,7 @@ Michel Ponceau
>> Nobuhiro Itou
>> Pete Vetere
>> Philippe Berthault
>> +Pieter Hollants
>> Saori Fukuta
>> Shigeki Sakamoto
>> Shuveb Hussain
> This is not necessary. AUTHORS file is now completely generated since
> v0.10.2-207-g7b21981.
>
>> diff --git a/docs/schemas/network.rng b/docs/schemas/network.rng
>> index 09d7c73..a2ce1c9 100644
>> --- a/docs/schemas/network.rng
>> +++ b/docs/schemas/network.rng
>> @@ -293,6 +293,12 @@
>>
>>
>>
>> +
>> +
>> +> name="unsignedByte"/>
>> +
Actually it appears that it's legal to send some options with no value
at all; this is even mentioned in dnsmasq documentation. So value needs
to be optional in the RNG, and in the parsing/formatting.
>> +
>> +
>>
>>
>>
> Maybe it's my lack of experience, but there's no mapping from numbers defined
> in RFC2132 to human readable strings and vice versa in my brain. I think,
> or value='1.2.3.4'/> is more readable than their numbered alternatives number='3' .../> and .
I think we can all agree on that. However, there is no official standard
alpha ID name for the options (although they're named in the rfc, it's
with a multi-word section title, not a single identifier", and dnsmasq
(at least) doesn't support names for all options. So we need to support
I am pleased to announce that release 1.0.1 of Sys-Virt, the libvirt
Perl API binding is now available for download:
http://search.cpan.org/CPAN/authors/id/D/DA/DANBERR/Sys-Virt-1.0.1.tar.gz
Changed in this release:
- Add all new APIs and constants in libvirt 1.0.1
- Fix typo preventing listing of NWFilters
- Add more testing of object list APIs
- Fix some incorrect error handling tests in binding
- Remove bogus compare < 0 for size_t variables
- Fix const-ness of functions for populating constants
- Add option to turn on more GCC warning flags
- Fix typos in POD docs
Further information, including online API documentation & previous release
archives, is available from the CPAN home page
http://search.cpan.org/dist/Sys-Virt/
Regards,
Daniel
--
|: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org -o- http://virt-manager.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/26/2013 09:27 AM, Michal Privoznik wrote:
> These two flags in fact are mutually exclusive. Requesting them both
> doesn't make any sense regardless of hypervisor driver. Hence, we have
> to make it within libvirt.c file instead of fixing it in each driver.
> ---
> src/libvirt.c | 52
> 1 file changed, 52 insertions(+)
ACK, and okay for 1.0.3.
--
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
These two flags in fact are mutually exclusive. Requesting them both
doesn't make any sense regardless of hypervisor driver. Hence, we have
to make it within libvirt.c file instead of fixing it in each driver.
---
src/libvirt.c | 52
1 file changed, 52 insertions(+)
diff --git a/src/libvirt.c b/src/libvirt.c
index 934997a..63416aa 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -5161,6 +5161,10 @@ virDomainMigrateDirect(virDomainPtr domain,
* XML includes details of the support URI schemes. If omitted
* the dconn will be asked for a default URI.
*
+ * If you want to copy non-shared storage within migration you
+ * can use either VIR_MIGRATE_NON_SHARED_DISK or
+ * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive.
+ *
* In either case it is typically only necessary to specify a
* URI if the destination host has multiple interfaces and a
* specific interface is required to transmit migration data.
@@ -5221,6 +5225,15 @@ virDomainMigrate(virDomainPtr domain,
goto error;
}
+if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
+flags & VIR_MIGRATE_NON_SHARED_INC) {
+virReportInvalidArg(flags,
+_("flags 'shared disk' and 'shared incremental' "
+ "in %s are mutually exclusive"),
+__FUNCTION__);
+goto error;
+}
+
if (flags & VIR_MIGRATE_OFFLINE) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
@@ -5375,6 +5388,10 @@ error:
* XML includes details of the support URI schemes. If omitted
* the dconn will be asked for a default URI.
*
+ * If you want to copy non-shared storage within migration you
+ * can use either VIR_MIGRATE_NON_SHARED_DISK or
+ * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive.
+ *
* In either case it is typically only necessary to specify a
* URI if the destination host has multiple interfaces and a
* specific interface is required to transmit migration data.
@@ -5448,6 +5465,15 @@ virDomainMigrate2(virDomainPtr domain,
goto error;
}
+if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
+flags & VIR_MIGRATE_NON_SHARED_INC) {
+virReportInvalidArg(flags,
+_("flags 'shared disk' and 'shared incremental' "
+ "in %s are mutually exclusive"),
+__FUNCTION__);
+goto error;
+}
+
if (flags & VIR_MIGRATE_OFFLINE) {
if (!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
@@ -5601,6 +5627,10 @@ error:
*
* VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set.
*
+ * If you want to copy non-shared storage within migration you
+ * can use either VIR_MIGRATE_NON_SHARED_DISK or
+ * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive.
+ *
* If a hypervisor supports renaming domains during migration,
* the dname parameter specifies the new name for the domain.
* Setting dname to NULL keeps the domain name the same. If domain
@@ -5648,6 +5678,15 @@ virDomainMigrateToURI(virDomainPtr domain,
virCheckNonNullArgGoto(duri, error);
+if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
+flags & VIR_MIGRATE_NON_SHARED_INC) {
+virReportInvalidArg(flags,
+_("flags 'shared disk' and 'shared incremental' "
+ "in %s are mutually exclusive"),
+__FUNCTION__);
+goto error;
+}
+
if (flags & VIR_MIGRATE_OFFLINE &&
!VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_MIGRATION_OFFLINE)) {
@@ -5741,6 +5780,10 @@ error:
*
* VIR_MIGRATE_TUNNELLED requires that VIR_MIGRATE_PEER2PEER be set.
*
+ * If you want to copy non-shared storage within migration you
+ * can use either VIR_MIGRATE_NON_SHARED_DISK or
+ * VIR_MIGRATE_NON_SHARED_INC as they are mutually exclusive.
+ *
* If a hypervisor supports changing the configuration of the guest
* during migration, the @dxml parameter specifies the new config
* for the guest. The configuration must include an identical set
@@ -5799,6 +5842,15 @@ virDomainMigrateToURI2(virDomainPtr domain,
goto error;
}
+if (flags & VIR_MIGRATE_NON_SHARED_DISK &&
+flags & VIR_MIGRATE_NON_SHARED_INC) {
+virReportInvalidArg(flags,
+_("flags 'shared disk' and 'shared incremental' "
+ "in %s are mutually exclusive"),
+__FUNCTION__);
+goto error;
+}
+
if (flags & VIR_MIGRATE_PEER2PEER) {
if (VIR_DRV_SUPPORTS_FEATURE(domain->conn->driver, domain->conn,
VIR_DRV_FEATURE_
Currently, if we label a file to match qemu process DAC label, we
do not store the original owner anywhere. So when relabeling
back, the only option we have is to relabel to root:root
which is obviously wrong.
However, bare remembering is not enough. We need to keep track of
how many times we labeled a file so only the last restore
chown()-s file back to the original owner.
In order to not pollute domain XML, this info is kept in driver's
private data in a hash table with path being key and pair
being value.
---
src/security/security_dac.c| 351 ++---
src/security/security_driver.h | 3 +
2 files changed, 296 insertions(+), 58 deletions(-)
diff --git a/src/security/security_dac.c b/src/security/security_dac.c
index 0b274b7..4b8f0a2 100644
--- a/src/security/security_dac.c
+++ b/src/security/security_dac.c
@@ -42,8 +42,121 @@ struct _virSecurityDACData {
uid_t user;
gid_t group;
bool dynamicOwnership;
+virHashTablePtr oldOwners; /* to hold pairs */
};
+struct _virOldLabel {
+char *owner;
+int refCount;
+};
+
+static void virOldLabelFree(virOldLabelPtr label)
+{
+if (!label)
+return;
+
+VIR_FREE(label->owner);
+VIR_FREE(label);
+}
+
+static void
+hashDataFree(void *payload, const void *name ATTRIBUTE_UNUSED)
+{
+virOldLabelFree(payload);
+}
+
+/**
+ * virSecurityDACRememberLabel:
+ * @priv: private DAC driver data
+ * @path: path which is about to be relabelled
+ *
+ * Store the original DAC label of @path.
+ * Returns: number of references of @path, or -1 on error
+ */
+static int
+virSecurityDACRememberLabel(virSecurityDACDataPtr priv,
+const char *path)
+{
+struct stat sb;
+virOldLabelPtr oldLabel = NULL;
+char *user = NULL, *group = NULL;
+
+oldLabel = virHashLookup(priv->oldOwners, path);
+
+if (oldLabel) {
+/* just increment ref counter */
+oldLabel->refCount++;
+goto cleanup;
+}
+
+if (stat(path, &sb) < 0) {
+virReportSystemError(errno, _("Unable to stat %s"), path);
+goto cleanup;
+}
+
+user = virGetUserName(sb.st_uid);
+group = virGetGroupName(sb.st_gid);
+if ((!user && (virAsprintf(&user, "+%ld", (long) sb.st_uid) < 0)) ||
+(!group && (virAsprintf(&group, "+%ld", (long) sb.st_gid) < 0)) ||
+(VIR_ALLOC(oldLabel) < 0) ||
+(virAsprintf(&oldLabel->owner, "%s:%s", user, group) < 0)) {
+virReportOOMError();
+VIR_FREE(oldLabel);
+goto cleanup;
+}
+
+oldLabel->refCount = 1;
+if (virHashUpdateEntry(priv->oldOwners, path, oldLabel) < 0) {
+virOldLabelFree(oldLabel);
+oldLabel = NULL;
+}
+
+cleanup:
+VIR_FREE(user);
+VIR_FREE(group);
+return oldLabel ? oldLabel->refCount : -1;
+}
+
+static int parseIds(const char *label, uid_t *uidPtr, gid_t *gidPtr);
+
+/**
+ * virSecurityDACGetRememberedLabel:
+ * @priv: private DAC driver data
+ * @path: path which we want to restore label on
+ * @user: original owner of @path
+ * @group: original owner of @path
+ *
+ * Decrements reference counter on @path. If this was the last
+ * reference, we need to restore the original label, in which
+ * case @user and @group are updated.
+ * Returns: the number of references of @path
+ * 0 if we need to restore the label
+ * -1 on error
+ */
+static int
+virSecurityDACGetRememberedLabel(virSecurityDACDataPtr priv,
+ const char *path,
+ uid_t *user,
+ gid_t *group)
+{
+int ret = -1;
+virOldLabelPtr oldLabel = NULL;
+
+oldLabel = virHashLookup(priv->oldOwners, path);
+if (!oldLabel)
+goto cleanup;
+
+ret = --oldLabel->refCount;
+
+if (!ret) {
+ret = parseIds(oldLabel->owner, user, group);
+virHashRemoveEntry(priv->oldOwners, path);
+}
+
+cleanup:
+return ret;
+}
+
void virSecurityDACSetUser(virSecurityManagerPtr mgr,
uid_t user)
{
@@ -242,14 +355,20 @@ virSecurityDACProbe(const char *virtDriver
ATTRIBUTE_UNUSED)
}
static int
-virSecurityDACOpen(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
+virSecurityDACOpen(virSecurityManagerPtr mgr)
{
-return 0;
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+
+priv->oldOwners = virHashCreate(0, hashDataFree);
+return priv->oldOwners ? 0 : -1;
}
static int
-virSecurityDACClose(virSecurityManagerPtr mgr ATTRIBUTE_UNUSED)
+virSecurityDACClose(virSecurityManagerPtr mgr)
{
+virSecurityDACDataPtr priv = virSecurityManagerGetPrivateData(mgr);
+
+virHashFree(priv->oldOwners);
return 0;
}
@@ -306,7 +425,9 @@ virSecurityDACSetOwnership(const char *path, uid_t uid,
gid_t gid)
}
static int
-virSecurityDACRestoreSecurityFileLabel(const char *path)
+virSecurityDACRestoreSecurityFileLabel(const char *path,
+
Just sending out early to make sure this time I am going in acceptable
direction before digging into selinux. Hopefully, apparmor won't be any deal as
I don't see anything that should be restored on domain shut off process.
Michal Privoznik (3):
security_dac: Remember owner prior chown() and restore on relabel
security_manager: Introduce {Save,Load}Status
security_dac: Implement {save,load}Status
src/lxc/lxc_controller.c | 2 +-
src/lxc/lxc_driver.c | 1 +
src/qemu/qemu_driver.c | 3 +
src/security/security_dac.c | 465 ++-
src/security/security_driver.h | 12 +
src/security/security_manager.c | 161 +-
src/security/security_manager.h | 2 +
tests/seclabeltest.c | 2 +-
tests/securityselinuxlabeltest.c | 3 +-
tests/securityselinuxtest.c | 3 +-
10 files changed, 590 insertions(+), 64 deletions(-)
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
In order to keep security driver's private data for future
libvirtd restart, we must copy the behaviour of other drivers and
create and load state XML file. This requires that the security
driver is able to produce its internal state in XML form and
parse it back then. These routines are called whenever security
manager API is called, because the manager can't know if the
driver's state has changed or not. The driver can decide to not
update the XML in which case saveStatus method should return 0
and set @buf argument to NULL.
The production of internal state XML can be enforced too. The
saveStatus method obtains @force argument set to true in which
case it should produce XML even though state hasn't change since
last run. This can be handy during migration when transferring
the internal state to the destination.
The state XML file is kept in the state directory of parent
hypervisor driver.
---
src/lxc/lxc_controller.c | 2 +-
src/lxc/lxc_driver.c | 1 +
src/qemu/qemu_driver.c | 3 +
src/security/security_driver.h | 9 +++
src/security/security_manager.c | 161 ++-
src/security/security_manager.h | 2 +
tests/seclabeltest.c | 2 +-
tests/securityselinuxlabeltest.c | 3 +-
tests/securityselinuxtest.c | 3 +-
9 files changed, 180 insertions(+), 6 deletions(-)
diff --git a/src/lxc/lxc_controller.c b/src/lxc/lxc_controller.c
index 15aa334..fe50c2a 100644
--- a/src/lxc/lxc_controller.c
+++ b/src/lxc/lxc_controller.c
@@ -1639,7 +1639,7 @@ int main(int argc, char *argv[])
ctrl->handshakeFd = handshakeFd;
if (!(ctrl->securityManager = virSecurityManagerNew(securityDriver,
-LXC_DRIVER_NAME,
+LXC_DRIVER_NAME, NULL,
false, false, false)))
goto cleanup;
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index f136df2..9da2bd9 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1387,6 +1387,7 @@ lxcSecurityInit(virLXCDriverPtr driver)
VIR_INFO("lxcSecurityInit %s", driver->securityDriverName);
virSecurityManagerPtr mgr =
virSecurityManagerNew(driver->securityDriverName,
LXC_DRIVER_NAME,
+ driver->stateDir,
false,
driver->securityDefaultConfined,
driver->securityRequireConfined);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1e96915..3b28a84 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -320,6 +320,7 @@ qemuSecurityInit(virQEMUDriverPtr driver)
while (names && *names) {
if (!(mgr = virSecurityManagerNew(*names,
QEMU_DRIVER_NAME,
+ cfg->stateDir,
cfg->allowDiskFormatProbing,
cfg->securityDefaultConfined,
cfg->securityRequireConfined)))
@@ -337,6 +338,7 @@ qemuSecurityInit(virQEMUDriverPtr driver)
} else {
if (!(mgr = virSecurityManagerNew(NULL,
QEMU_DRIVER_NAME,
+ cfg->stateDir,
cfg->allowDiskFormatProbing,
cfg->securityDefaultConfined,
cfg->securityRequireConfined)))
@@ -348,6 +350,7 @@ qemuSecurityInit(virQEMUDriverPtr driver)
if (cfg->privileged) {
if (!(mgr = virSecurityManagerNewDAC(QEMU_DRIVER_NAME,
+ cfg->stateDir,
cfg->user,
cfg->group,
cfg->allowDiskFormatProbing,
diff --git a/src/security/security_driver.h b/src/security/security_driver.h
index d54f754..ee255a0 100644
--- a/src/security/security_driver.h
+++ b/src/security/security_driver.h
@@ -112,6 +112,12 @@ typedef int (*virSecurityDomainSetHugepages)
(virSecurityManagerPtr mgr,
virDomainDefPtr def,
const char *path);
+typedef int (*virSecurityDriverSaveStatus) (virSecurityManagerPtr mgr,
+virBufferPtr *buf,
+bool force);
+typedef int (*virSecurityDriverLoadStatus) (virSecurityManagerPtr mgr,
+xmlDocPtr xml);
On 02/25/2013 03:31 PM, Peter Krempa wrote:
> Qemu's implementation of virtio RNG supports rate limiting of the
> entropy used. This patch exposes the option to tune this fucntionality.
s/fucntionality/functionality/
>
> This patch is based on qemu commit 904d6f588063fb5ad2b61998acdf1e73fb4
>
> The rate limiting is exported in the XML as:
>
> ...
>
> 4321
>
>
> ...
> ---
>
> Notes:
> This series would benefit from the per-driver XML parsing checks to verify
> that rate > 8bits, otherwise it will be rounded down to 0 bytes. I will
> follow up with that change as soon as the per-driver callbacks get in.
>
> Version 3:
> - State the time unit in docs
> Version 2:
> - Qemu uses bytes/period, adapt the value according to that
>
> docs/formatdomain.html.in | 10 ++
> docs/schemas/domaincommon.rng | 18
> +-
> src/conf/domain_conf.c | 17 +
> src/conf/domain_conf.h | 2 ++
> src/qemu/qemu_command.c| 9 +
> .../qemuxml2argv-virtio-rng-random.args| 2 +-
> .../qemuxml2argv-virtio-rng-random.xml | 1 +
> 7 files changed, 57 insertions(+), 2 deletions(-)
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 2a60f61..220884c 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -4294,6 +4294,7 @@ qemu-kvm -net nic,model=? /dev/null
>...
>
>
> + 1234
>/dev/random
>
>
> @@ -4316,6 +4317,15 @@ qemu-kvm -net nic,model=? /dev/null
>'virtio' — supported by qemu and virtio-rng kernel
> module
>
>
> + rate
> +
> +
> + The rate element allows to limit the rate that the entropy can be
grammar doesn't quite flow here; see below
> + read from the source. The value is in bits that the device is
> allowed
> + to read in the selected period. The period is represented in
> miliseconds.
s/miliseconds/milliseconds/
> + The default period is 1000ms or 1 second.
I'm still not sure we accurately covered things. Maybe:
The optional rate element allows limiting the rate at which
entropy can be consumed from the source. An optional
period attribute specifies the duration of a period in
milliseconds; if omitted, the period is taken as 1000 milliseconds (1
second). The element contents specify how many bits are permitted per
period. Drivers may enforce a minimum rate, and may round the rate down
to a minimum granularity.
> +++ b/src/conf/domain_conf.h
> @@ -1736,6 +1736,8 @@ enum virDomainRNGBackend {
> struct _virDomainRNGDef {
> int model;
> int backend;
> +unsigned int rate;
> +unsigned int period;
Comments here would help:
unsigned int rate; /* bits per period */
unsigned int period; /* milliseconds */
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.args
> @@ -1 +1 @@
> -LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S
> -M pc -m 214 -smp 1 -nographic -nodefaults -monitor
> unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -device
> virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 -object
> rng-random,id=rng0,filename=/test/phile -device
> virtio-rng-pci,rng=rng0,bus=pci.0,addr=0x4
> +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/bin/qemu -S
> -M pc -m 214 -smp 1 -nographic -nodefaults -monitor
> unix:/tmp/test-monitor,server,nowait -no-acpi -boot c -usb -device
> virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3 -object
> rng-random,id=rng0,filename=/test/phile -device
> virtio-rng-pci,rng=rng0,max-bytes=100,period=1234,bus=pci.0,addr=0x4
This is a long line; please split it with a backslash-newline to fit in
80 columns.
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
> b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
> index ab1f38c..26ddd38 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-virtio-rng-random.xml
> @@ -17,6 +17,7 @@
>
>
>
> + 800
>/test/phile
>
>
>
--
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
Hi,
Am 26.02.2013 um 13:06 schrieb Philipp Hahn :
> @Sebastian: Is (uid_t)-1 = (u32)-1 special for Sheepdog or was the file just
> created by a previous virsh-dump, which outputs UINT_MAX instead of -1?
The sheepdog backend doesn't care about uid/gid. I probably created the file
using virsh-dump.
Best regards,
Sebastian
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/25/13 23:31, Peter Krempa wrote:
qemu supports adding multiple RNG devices. This patch allows libvirt to
support this.
This patch isn't relevant anymore. Qemu supports adding multiple RNG
devices but it apparently shouldn't and the support will be limited in
the next release.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Il 26/02/2013 15:04, Eduardo Habkost ha scritto:
>> > For me, the INI way to do multi-valued keys is still fine.
> Having multiple-valued keys (cpus=A,cpus=B,cpus=C) seems like the best
> intermediate solution while we don't have a decent generic syntax.
Even more so since:
1) we already support it for -net;
2) our config file format is not a random INI variant, it's explicitly
based on git's config file format, and it supports multiple-valued keys.
For example here is a stanza of my .git/config file.
[remote "mirror"]
url = git://github.com/bonzini/qemu.git
pushurl = g...@github.com:bonzini/qemu.git
fetch = +refs/heads/*:refs/remotes/mirror/*
push = +refs/heads/*:refs/heads/*
push = +refs/heads/master:refs/heads/integration
push = +refs/remotes/origin/master:refs/heads/master
push = +refs/tags/*:refs/tags/*
Paolo
> Except that Anthony doesn't like it.
> Anthony, care to explain why exactly you don't want it?
>
>
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
v3->v2 difference: Reduced the amount of change to a few comments and
adjusting the (NULL == xxx) and (-1 == xxx) checks
Since these are just documentation changes - once ACK'd is it OK to push
now or should I wait for after the freeze?
Tks,
John
---
examples/hellolibvirt/hellolibvirt.c | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/examples/hellolibvirt/hellolibvirt.c
b/examples/hellolibvirt/hellolibvirt.c
index 234637e..335a75e 100644
--- a/examples/hellolibvirt/hellolibvirt.c
+++ b/examples/hellolibvirt/hellolibvirt.c
@@ -1,5 +1,6 @@
/* This file contains trivial example code to connect to the running
- * hypervisor and gather a few bits of information. */
+ * hypervisor and gather a few bits of information about domains.
+ * Similar API's exist for storage pools, networks, and interfaces. */
#include
@@ -15,7 +16,7 @@ showError(virConnectPtr conn)
virErrorPtr err;
err = malloc(sizeof(*err));
-if (NULL == err) {
+if (!err) {
printf("Could not allocate memory for error data\n");
goto out;
}
@@ -56,7 +57,7 @@ showHypervisorInfo(virConnectPtr conn)
* to fail if, for example, there is no connection to a
* hypervisor, so check what it returns. */
hvType = virConnectGetType(conn);
-if (NULL == hvType) {
+if (!hvType) {
ret = 1;
printf("Failed to get hypervisor type\n");
showError(conn);
@@ -93,7 +94,7 @@ showDomains(virConnectPtr conn)
char **nameList = NULL;
numActiveDomains = virConnectNumOfDomains(conn);
-if (-1 == numActiveDomains) {
+if (numActiveDomains == -1) {
ret = 1;
printf("Failed to get number of active domains\n");
showError(conn);
@@ -101,7 +102,7 @@ showDomains(virConnectPtr conn)
}
numInactiveDomains = virConnectNumOfDefinedDomains(conn);
-if (-1 == numInactiveDomains) {
+if (numInactiveDomains == -1) {
ret = 1;
printf("Failed to get number of inactive domains\n");
showError(conn);
@@ -113,17 +114,20 @@ showDomains(virConnectPtr conn)
nameList = malloc(sizeof(*nameList) * numInactiveDomains);
-if (NULL == nameList) {
+if (!nameList) {
ret = 1;
printf("Could not allocate memory for list of inactive domains\n");
goto out;
}
+/* The virConnectListDomains() will return a list of the active domains.
+ * Alternatively the virConnectListAllDomains() API would return a list
+ * of both active and inactive domains */
numNames = virConnectListDefinedDomains(conn,
nameList,
numInactiveDomains);
-if (-1 == numNames) {
+if (numNames == -1) {
ret = 1;
printf("Could not get list of defined domains from hypervisor\n");
showError(conn);
@@ -136,9 +140,7 @@ showDomains(virConnectPtr conn)
for (i = 0 ; i < numNames ; i++) {
printf(" %s\n", *(nameList + i));
-/* The API documentation doesn't say so, but the names
- * returned by virConnectListDefinedDomains are strdup'd and
- * must be freed here. */
+/* must free the returned named per the API documentation */
free(*(nameList + i));
}
@@ -163,7 +165,7 @@ main(int argc, char *argv[])
* except, possibly, the URI of the hypervisor. */
conn = virConnectOpenAuth(uri, virConnectAuthPtrDefault, 0);
-if (NULL == conn) {
+if (!conn) {
ret = 1;
printf("No connection to hypervisor\n");
showError(conn);
@@ -171,7 +173,7 @@ main(int argc, char *argv[])
}
uri = virConnectGetURI(conn);
-if (NULL == uri) {
+if (!uri) {
ret = 1;
printf("Failed to get URI for hypervisor connection\n");
showError(conn);
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Update the function prototypes to include a message about the client needing
to free() returned name fields. Fix the all domains example flags values.
v3->v2 diff:
Separted this out to it's own patch.
---
src/libvirt.c | 21 ++---
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 934997a..8447c0e 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -8304,19 +8304,18 @@ error:
* VIR_CONNECT_LIST_DOMAINS_NO_SNAPSHOT, for filtering based on whether
* a domain has snapshots.
*
- * Returns the number of domains found or -1 and sets domains to NULL in case
- * of error. On success, the array stored into @doms is guaranteed to have an
+ * Returns the number of domains found or -1 and sets domains to NULL in case
of
+ * error. On success, the array stored into @domains is guaranteed to have an
* extra allocated element set to NULL but not included in the return count, to
* make iteration easier. The caller is responsible for calling virDomainFree()
- * on each array element, then calling free() on @doms.
+ * on each array element, then calling free() on @domains.
*
* Example of usage:
* virDomainPtr *domains;
- * virDomainPtr dom;
* int i;
* int ret;
- * unsigned int flags = VIR_CONNECT_LIST_RUNNING |
- * VIR_CONNECT_LIST_PERSISTENT;
+ * unsigned int flags = VIR_CONNECT_LIST_DOMAINS_RUNNING |
+ * VIR_CONNECT_LIST_DOMAINS_PERSISTENT;
*
* ret = virConnectListAllDomains(conn, &domains, flags);
* if (ret < 0)
@@ -10226,7 +10225,7 @@ error:
* this command is inherently racy; a network can be started between a call
* to virConnectNumOfNetworks() and this call; you are only guaranteed that
* all currently active networks were listed if the return is less than
- * @maxnames.
+ * @maxnames. The client must call free() on each returned name.
*/
int
virConnectListNetworks(virConnectPtr conn, char **const names, int maxnames)
@@ -11208,7 +11207,7 @@ error:
* this command is inherently racy; a interface can be started between a call
* to virConnectNumOfInterfaces() and this call; you are only guaranteed that
* all currently active interfaces were listed if the return is less than
- * @maxnames.
+ * @maxnames. The client must call free() on each returned name.
*/
int
virConnectListInterfaces(virConnectPtr conn, char **const names, int maxnames)
@@ -12076,7 +12075,7 @@ error:
* this command is inherently racy; a pool can be started between a call to
* virConnectNumOfStoragePools() and this call; you are only guaranteed
* that all currently active pools were listed if the return is less than
- * @maxnames.
+ * @maxnames. The client must call free() on each returned name.
*/
int
virConnectListStoragePools(virConnectPtr conn,
@@ -18440,7 +18439,7 @@ error:
* the results, see virDomainListAllSnapshots().
*
* Returns the number of domain snapshots found or -1 in case of error.
- * The caller is responsible for freeing each member of the array.
+ * The caller is responsible to call free() for each member of the array.
*/
int
virDomainSnapshotListNames(virDomainPtr domain, char **names, int nameslen,
@@ -18689,7 +18688,7 @@ error:
* the results, see virDomainSnapshotListAllChildren().
*
* Returns the number of domain snapshots found or -1 in case of error.
- * The caller is responsible for freeing each member of the array.
+ * The caller is responsible to call free() for each member of the array.
*/
int
virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
--
1.7.11.7
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Feb 26, 2013 at 02:58:19PM +0100, Ján Tomko wrote:
> Right now, we allocate a port or a TLS port for SPICE if it's set to -1,
> even if autoport is off. But we only free them if autoport is on.
>
> With autoport on, we only allocate TLS port if cfg->spiceTLS is set, but
> we free it even if it's not, leading to an error message.
>
> This patch separates the autoport=yes|no XML option into autoport and
> tlsAutoport in virDomainGraphicsDef:
> if autoport is yes, we set them both to 1 (and vice versa)
> if either of the port numbers is -1, the corresponding autoport is set
> to 1 and it's used as a condition for acquiring/releasing the port.
>
> For TLS ports, cfg->spiceTLS is checked before releasing the port as
> well.
>
> Also check the port number before trying to release it - the vnc port
> will be 0, the spice port will be -1 or 0 if it hasn't been allocated
> yet (if qemuProcessStart fails before port allocation).
> ---
> src/conf/domain_conf.c | 17 +++--
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_hotplug.c | 3 ++-
> src/qemu/qemu_process.c | 26 ++
> 4 files changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index b65e52a..cb27f82 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -7083,8 +7083,10 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
> }
>
> if ((autoport = virXMLPropString(node, "autoport")) != NULL) {
> -if (STREQ(autoport, "yes"))
> +if (STREQ(autoport, "yes")) {
> def->data.spice.autoport = 1;
> +def->data.spice.tlsAutoport = 1;
> +}
> VIR_FREE(autoport);
> }
>
> @@ -7102,12 +7104,14 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
> VIR_FREE(defaultMode);
> }
>
> -if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) {
> -/* Legacy compat syntax, used -1 for auto-port */
> +/* Legacy compat syntax, used -1 for auto-port */
> +if (def->data.spice.port == -1)
> def->data.spice.autoport = 1;
> -}
> +if (def->data.spice.tlsPort == -1)
> +def->data.spice.tlsAutoport = 1;
>
> -if (def->data.spice.autoport && (flags & VIR_DOMAIN_XML_INACTIVE)) {
> +if (def->data.spice.autoport && def->data.spice.tlsAutoport &&
> +(flags & VIR_DOMAIN_XML_INACTIVE)) {
> def->data.spice.port = 0;
> def->data.spice.tlsPort = 0;
> }
> @@ -14201,7 +14205,8 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
>def->data.spice.tlsPort);
>
> virBufferAsprintf(buf, " autoport='%s'",
> - def->data.spice.autoport ? "yes" : "no");
> + (def->data.spice.autoport &&
> + def->data.spice.tlsAutoport) ? "yes" : "no");
>
> if (listenAddr)
> virBufferAsprintf(buf, " listen='%s'", listenAddr);
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 0828954..cc67716 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -1356,6 +1356,7 @@ struct _virDomainGraphicsDef {
> char *keymap;
> virDomainGraphicsAuthDef auth;
> unsigned int autoport :1;
> +unsigned int tlsAutoport :1;
> int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST];
> int defaultMode; /* enum virDomainGraphicsSpiceChannelMode */
> int image;
NACK to these changes to domain_conf. The bug is in the QEMU code,
so the fix should be done in the QEMU code without introducing
this extra 'tlsAutoport' field that isn't in the XML anywhere.
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
On Tue, Feb 26, 2013 at 11:50:08AM +0100, Markus Armbruster wrote:
> Eduardo Habkost writes:
>
> > This allows ":" to be used a separator between each CPU range, so the
> > command-line may look like:
> >
> > -numa node,cpus=A-B:C-D
> >
> > Note that the following format, currently used by libvirt:
> >
> > -numa nodes,cpus=A-B,C-D
> >
> > will _not_ work, as "," is the option separator for the command-line
> > option parser, and it would require changing the -numa option parsing
> > code to handle "cpus" as a special case.
> >
> > Signed-off-by: Eduardo Habkost
> > ---
> > Changes v2:
> > - Use ":" as separator
> > - Document the new format
>
> See also discussion on multi-valued keys in command line option
> arguments and config files in v1 thread. Hopefully we can reach a
> conclusion soon, and then we'll see whether this patch is what we want.
Yeah, let's drop this patch by now. I am starting to be convinced that
"cpus=A,cpus=B,cpus=C" is the best approach. It is not pretty, but at
least it uses generic parser code instead of yet another ad-hoc parser.
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/26/13 13:45, Eric Blake wrote:
> On 02/26/2013 05:05 AM, Ján Tomko wrote:
>> VIR_ERR_NO_CONNECT already contains "no connection driver available".
>>
>> This patch changes:
>> no connection driver available for No connection for URI hello
>> to:
>> no connection driver available for hello
>>
>> Bug: https://bugzilla.redhat.com/show_bug.cgi?id=851413
>> ---
>> src/libvirt.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> ACK.
>
Thank you. I've pushed it now.
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Tue, Feb 26, 2013 at 10:53:07AM +0100, Markus Armbruster wrote:
> Eduardo Habkost writes:
>
> > On Thu, Feb 21, 2013 at 09:23:22PM +0100, Markus Armbruster wrote:
> >> Eduardo Habkost writes:
> >>
> >> > This allows "," to be used a separator between each CPU range. Note
> >> > that commas inside key=value command-line options have to be escaped
> >> > using ",,", so the command-line will look like:
> >> >
> >> > -numa node,cpus=A,,B,,C,,D
> >>
> >> This is really, really ugly, and an embarrassment to document. Which
> >> you didn't ;)
> >
> > I was trying to have an intermediate solution using the current -numa
> > parser. I have patches in my queue that will change the code to properly
> > use QemuOpts later.
> >
> > It would be interesting to support the "A,B,C,D" format in config files,
> > though, as it is simple and straighforward when no escaping is required.
>
> Our config file syntax is in a Windows INI dialect: key=value lines
> grouped into sections. Our dialect requires values to be enclosed in
> quotes. Commonly, the quotes are optional. Could be fixed. It
> supports multi-valued keys the common INI way: multiple key=value lines
> for the same key, one per value
>
> key = "A,B,C" works when the A, B, C can't contain commas. Fine for a
> list of numbers. For long lists, we'd probably want to add a line
> continuation feature.
>
> Strings can contain commas, so you'd have to do something like key =
> "A", "B", "C". Whether that's still Windows INI is debatable. More so
> since there's already a common way to do it: one line per value.
I was only thinking about the -numa option problem. Having a more
generic solution would surely be even better.
>
> If we decide INI doesn't meet our needs or desires for pretty syntax, we
> should not extend it beyond its limits into QEMU's very own
> configuration syntax. We should switch to a common syntax that serves
> our needs and desires. For what it's worth, we already parse JSON.
I completely agree. But by now I just want to know what we should do
while we don't have a generic parser/syntax that can handle lists in a
pretty way. So:
>
> For me, the INI way to do multi-valued keys is still fine.
Having multiple-valued keys (cpus=A,cpus=B,cpus=C) seems like the best
intermediate solution while we don't have a decent generic syntax.
Except that Anthony doesn't like it.
Anthony, care to explain why exactly you don't want it?
>
> >> What about
> >>
> >> -numa node,cpus=A,cpus=B,cpus=C,cpus=D
> >
> > Looks better for the command-line usage, at least. I will give it a try.
> >
> >>
> >> Yes, QemuOpts lets you do that. Getting all the values isn't as easy as
> >> it could be (unless you use Laszlo's opt-visitor), but that could be
> >> improved.
> >
> > Guess what: -numa doesn't even use QemuOpts, and I am not sure the
> > current format of -numa will allow QemuOpts to be used easily. I expect
> > the proper solution using QemuOpts to involve having a
> > standards-compliant "numa-node" config section instead of this weird
> > "-numa ,..." format where the only valid that ever existed
> > was "node".
>
> This is the current -numa syntax, as far as I can tell:
>
> -numa node,KEY=VALUE,...
>
> Recognized KEY=VALUE:
>
> nodeid=UINT
> mem=SIZE
> cpus=[|UINT|UINT-UINT]
>
> Unrecognized KEYs are silently ignored.
>
> This should fit into QemuOpts just fine. Sketch:
>
> static QemuOptsList qemu_numa_opts = {
> .name = "numa",
> .implied_opt_name = "type"
> .head = QTAILQ_HEAD_INITIALIZER(qemu_rtc_numa.head),
> .desc = {
> {
> .name = "type",
> .type = QEMU_OPT_STRING,
> .help = "node type"
> },
The "node" part is not a "node type", it is an "numa option type", and
the only valid "option type" today is "node" (which is what makes the
current syntax seem weird to me).
I would simply drop the "numa" part from the command-line argument and
name the new config section "numa-node". I will send patches to do that,
later.
> {
> .name = "nodeid",
> .type = QEMU_OPT_NUMBER,
> .help = "node ID"
> }, {
> .name = "mem",
> .type = QEMU_OPT_SIZE,
> .help = "memory size"
> }, {
I need to double-check that QEMU_OPT_SIZE has exactly the same behavior
of the ad-hoc parser, first.
> .name = "cpus",
> .type = QEMU_OPT_STRING,
> .help = "CPU range"
> },
> { /* end of list */ }
> },
> };
>
>
> type = qemu_opt_get(opts);
> if (!type || strcmp(type, "node)) {
> // error
> }
> // get and record nodeid, mem
> // get, parse and record cpus
>
> This rejects unrecognized keys, unlike the current code. Declare bug
> fix ;)
Good. :-)
>
> To support discontinuous CPU sets, simply get all values of key "cpus".
I think I have an unfinished work branch that did that. But Paolo also
have a
Right now, we allocate a port or a TLS port for SPICE if it's set to -1,
even if autoport is off. But we only free them if autoport is on.
With autoport on, we only allocate TLS port if cfg->spiceTLS is set, but
we free it even if it's not, leading to an error message.
This patch separates the autoport=yes|no XML option into autoport and
tlsAutoport in virDomainGraphicsDef:
if autoport is yes, we set them both to 1 (and vice versa)
if either of the port numbers is -1, the corresponding autoport is set
to 1 and it's used as a condition for acquiring/releasing the port.
For TLS ports, cfg->spiceTLS is checked before releasing the port as
well.
Also check the port number before trying to release it - the vnc port
will be 0, the spice port will be -1 or 0 if it hasn't been allocated
yet (if qemuProcessStart fails before port allocation).
---
src/conf/domain_conf.c | 17 +++--
src/conf/domain_conf.h | 1 +
src/qemu/qemu_hotplug.c | 3 ++-
src/qemu/qemu_process.c | 26 ++
4 files changed, 28 insertions(+), 19 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b65e52a..cb27f82 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -7083,8 +7083,10 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
}
if ((autoport = virXMLPropString(node, "autoport")) != NULL) {
-if (STREQ(autoport, "yes"))
+if (STREQ(autoport, "yes")) {
def->data.spice.autoport = 1;
+def->data.spice.tlsAutoport = 1;
+}
VIR_FREE(autoport);
}
@@ -7102,12 +7104,14 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
VIR_FREE(defaultMode);
}
-if (def->data.spice.port == -1 && def->data.spice.tlsPort == -1) {
-/* Legacy compat syntax, used -1 for auto-port */
+/* Legacy compat syntax, used -1 for auto-port */
+if (def->data.spice.port == -1)
def->data.spice.autoport = 1;
-}
+if (def->data.spice.tlsPort == -1)
+def->data.spice.tlsAutoport = 1;
-if (def->data.spice.autoport && (flags & VIR_DOMAIN_XML_INACTIVE)) {
+if (def->data.spice.autoport && def->data.spice.tlsAutoport &&
+(flags & VIR_DOMAIN_XML_INACTIVE)) {
def->data.spice.port = 0;
def->data.spice.tlsPort = 0;
}
@@ -14201,7 +14205,8 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
def->data.spice.tlsPort);
virBufferAsprintf(buf, " autoport='%s'",
- def->data.spice.autoport ? "yes" : "no");
+ (def->data.spice.autoport &&
+ def->data.spice.tlsAutoport) ? "yes" : "no");
if (listenAddr)
virBufferAsprintf(buf, " listen='%s'", listenAddr);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 0828954..cc67716 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1356,6 +1356,7 @@ struct _virDomainGraphicsDef {
char *keymap;
virDomainGraphicsAuthDef auth;
unsigned int autoport :1;
+unsigned int tlsAutoport :1;
int channels[VIR_DOMAIN_GRAPHICS_SPICE_CHANNEL_LAST];
int defaultMode; /* enum virDomainGraphicsSpiceChannelMode */
int image;
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 78961a7..d8f9007 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -1879,9 +1879,10 @@ qemuDomainChangeGraphics(virQEMUDriverPtr driver,
case VIR_DOMAIN_GRAPHICS_TYPE_SPICE:
if ((olddev->data.spice.autoport != dev->data.spice.autoport) ||
+(olddev->data.spice.tlsAutoport != dev->data.spice.tlsAutoport) ||
(!dev->data.spice.autoport &&
(olddev->data.spice.port != dev->data.spice.port)) ||
-(!dev->data.spice.autoport &&
+(!dev->data.spice.tlsAutoport &&
(olddev->data.spice.tlsPort != dev->data.spice.tlsPort))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("cannot change port settings on spice graphics"));
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 732964f..7c2a54e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3622,8 +3622,7 @@ int qemuProcessStart(virConnectPtr conn,
graphics->data.vnc.port = port;
} else if (graphics->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
unsigned short port = 0;
-if (graphics->data.spice.autoport ||
-graphics->data.spice.port == -1) {
+if (graphics->data.spice.autoport) {
if (virPortAllocatorAcquire(driver->remotePorts, &port) < 0)
goto cleanup;
@@ -3635,9 +3634,7 @@ int qemuProcessStart(virConnectPtr conn,
graphics->data.spice.port = port;
use two blank line before the next , which helps reading the source
file.
I find the html file still hard to read as there is no clear separation
between different storage pool types. Some CSS tweaks would be
appreciated.
Signed-off-by: Philipp Hahn
---
docs/storage.html.in |6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/docs/storage.html.in b/docs/storage.html.in
index 8391f85..14eba20 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -115,6 +115,7 @@
+
Directory pool
A pool with a type of dir provides the means to manage
@@ -165,7 +166,6 @@
either qemu-img or qcow-create tools
are present. The others are dependent on support of the
qemu-img tool.
-
Filesystem pool
@@ -455,6 +455,7 @@
The iSCSI volume pool does not use the volume format type element.
+
SCSI volume pools
This provides a pool based on a SCSI HBA. Volumes are preexisting SCSI
@@ -487,6 +488,7 @@
The SCSI volume pool does not use the volume format type element.
+
Multipath pools
This provides a pool that contains all the multipath devices on the
@@ -519,6 +521,7 @@
The Multipath volume pool does not use the volume format type element.
+
RBD pools
This storage driver provides a pool which contains all RBD
@@ -590,6 +593,7 @@
The RBD pool does not use the volume format type element.
+
Sheepdog pools
This provides a pool based on a Sheepdog Cluster.
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Feb 25, 2013 at 10:04:07PM +0100, Andreas Färber wrote:
> Am 21.02.2013 21:57, schrieb Eduardo Habkost:
> > On Thu, Feb 21, 2013 at 09:23:22PM +0100, Markus Armbruster wrote:
> >> Eduardo Habkost writes:
> >>
> >>> This allows "," to be used a separator between each CPU range. Note
> >>> that commas inside key=value command-line options have to be escaped
> >>> using ",,", so the command-line will look like:
> >>>
> >>> -numa node,cpus=A,,B,,C,,D
> >>
> >> This is really, really ugly, and an embarrassment to document. Which
> >> you didn't ;)
> >
> > I was trying to have an intermediate solution using the current -numa
> > parser. I have patches in my queue that will change the code to properly
> > use QemuOpts later.
>
> Speaking of which, have you considered using QemuOpts for -cpu? Its
> custom parsing code will probably not handle , escaping at all. ;)
It may be possible, but I'm not sure QemuOpts can handle the "+foo,-foo"
options (and I am sure we don't want to extend QemuOpts to support
them).
In either case, it's better to do that after we simplify
x86_cpu_parse_featurestr() (with the current patches from Igor), to make
the conversion easier to review later.
--
Eduardo
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
QEMU should be written all upper case.
Signed-off-by: Philipp Hahn
---
docs/storage.html.in | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/docs/storage.html.in b/docs/storage.html.in
index 930d603..8391f85 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -524,13 +524,13 @@
This storage driver provides a pool which contains all RBD
images in a RADOS pool. RBD (RADOS Block Device) is part
of the Ceph distributed storage project.
- This backend only supports Qemu with RBD support. Kernel RBD
+ This backend only supports QEMU with RBD support. Kernel RBD
which exposes RBD devices as block devices in /dev is not
supported. RBD images created with this storage backend
can be accessed through kernel RBD if configured manually, but
this backend does not provide mapping for these images.
- Images created with this backend can be attached to Qemu guests
- when Qemu is build with RBD support (Since Qemu 0.14.0). The
+ Images created with this backend can be attached to QEMU guests
+ when QEMU is build with RBD support (Since QEMU 0.14.0). The
backend supports cephx authentication for communication with the
Ceph cluster. Storing the cephx authentication key is done with
the libvirt secret mechanism. The UUID in the example pool input
@@ -574,7 +574,7 @@
Example disk attachement
-RBD images can be attached to Qemu guests when Qemu is built
+RBD images can be attached to QEMU guests when QEMU is built
with RBD support. Information about attaching a RBD image to a
guest can be found
at format domain
@@ -633,7 +633,7 @@
Example disk attachment
-Sheepdog images can be attached to Qemu guests.
+Sheepdog images can be attached to QEMU guests.
Information about attaching a Sheepdog image to a
guest can be found
at the format domain
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Add qed for dirfs pool.
Add ocfs2 for disk pool.
Add lvm2 for disk and logical pool.
Add cifs and glusterfs for netfs pool.
Note: POOL_DISK_LVM2 can not be created by "parted mklabel", but is only
returned from auto-detection on disk pools.
Signed-off-by: Philipp Hahn
---
docs/storage.html.in | 20 +++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/docs/storage.html.in b/docs/storage.html.in
index 26a949a..92d7842 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -153,6 +153,7 @@
iso: CDROM disk image format
qcow: QEMU v1 disk image format
qcow2: QEMU v2 disk image format
+ qed: QEMU Enhanced Disk image format
vmdk: VMWare disk image format
vpc: VirtualPC disk image format
@@ -229,6 +230,9 @@
xfs
+
+ocfs2
+
Valid volume format types
@@ -269,6 +273,12 @@
nfs
+
+glusterfs
+
+
+cifs
+
Valid volume format types
@@ -304,8 +314,13 @@
Valid pool format types
- The logical volume pool does not use the pool format type element.
+ The logical volume pool supports the following formats:
+
+ auto - automatically determine format
+
+lvm2
+
Valid volume format types
@@ -361,6 +376,9 @@
sun
+
+lvm2
+
The dos or gpt formats are recommended for
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Hello,
some documentation updates for new storage pools and volumes.
Philipp Hahn (4):
doc: document new storage volume/pool types
doc: add storage format entries
doc: fix writing of QEMU
doc: white space changes
docs/storage.html.in | 40 +---
1 file changed, 33 insertions(+), 7 deletions(-)
--
1.7.10.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Hi all,
In this function qemuMonitorJSONExtractCPUInfo(),
It assumes that cpu index is always contiguous.
if (cpu != i) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("unexpected cpu index %d expecting %d"),
i, cpu);
goto cleanup;
}
For x86, in what kind of situation does this error happen?
This assumption causes the problem for ppc64.
For ppc64, it's quite different from x86.
SMT is needed to be disabled because of hardware's limitation.
So, only one thread per core is online.
These threads are not contiguous.
For example, smt=4, 64 CPUs and 16 Cores
CPU information is as the following:
Architecture: ppc64
Byte Order:Big Endian
CPU(s):64
On-line CPU(s) list: 0,4,8,12,16,20,24,28,32,36,40,44,48,52,56,60
Off-line CPU(s) list:
1-3,5-7,9-11,13-15,17-19,21-23,25-27,29-31,33-35,37-39,41-43,45-47,49-51,53-55,57-59,61-63
Thread(s) per core:1
Core(s) per socket:1
Socket(s): 16
NUMA node(s): 2
All CPUs we get are only 0,4, ..., 60.
If possible, can this assumption be removed?
Thanks.
--
Li Zhang
IBM China Linux Technology Centre
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Feb 25, 2013 at 17:38:32 +0100, Paolo Bonzini wrote:
> Currently we call virSetDeviceUnprivSGIO with val == 0 if a block device
> has an sgio attribute. But for sgio='filtered', we know that a
> kernel with no unpriv_sgio support will always behave as the user
> wanted. In this case, there is no need to call the function and
> report a (bogus) error.
>
> Signed-off-by: Paolo Bonzini
> ---
> src/qemu/qemu_process.c | 35 +--
> 1 file changed, 17 insertions(+), 18 deletions(-)
ACK and pushed. Thanks,
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Hello,
Am Montag 25 Februar 2013, 15:58:50 schrieb Michal Privoznik:
> On 22.02.2013 17:55, Philipp Hahn wrote:
> > uid_t and gid_t are opaque types, ranging from s32 to u32 to u64.
...
> This one breaks storagevolxml2xmltest:
> TEST: storagevolxml2xmltest
...
> ... OK 7) Storage Vol XML-2-XML vol-sheepdog
> ... Offset 283
> Expect [4294967295
> 4294967295]
> Actual [-1
> -1]
> ...
> FAILED
The attached 1st patch would change the test case, but since I don't know
anything about sheepdog, I can't say it that is really the correct change.
@Sebastian: Is (uid_t)-1 = (u32)-1 special for Sheepdog or was the file just
created by a previous virsh-dump, which outputs UINT_MAX instead of -1?
There is one more interesting case in storagepoolxml2xmlout/pool-dir.xml,
which should currently fail on 32 bit compiles, but does not, because pool-
dumpxml only returns the original pool-xml with user/group=-1 (and updates
capacities).
> However, the first 3 patches are correct. Even though it's freeze, they are
> pure bugfixes so I've pushed them.
Thanks.
On Monday 25 February 2013 19:46:50 Eric Blake wrote:
>I like the standardized name INT_MAX better than the invented name ALLONE.
Better, but not correct: UINT_MAX is what is required here, because INT_MAX is
the signed one with MSB=0.
>> +double d;
>
>Eww - why do we need to use a double?
Ask libxml2/libxml/xpath.h:
>struct _xmlXPathObject {
>xmlXPathObjectType type;
>xmlNodeSetPtr nodesetval;
>int boolval;
>double floatval;
>xmlChar *stringval;
>void *user;
>int index;
>void *user2;
>int index2;
>};
The xpath expression "Number(./owner)" must work for integer and floating point
values, so a double is returned by "Number()".
If you take a look at virXPathLongBase(), you'll see no such "double", because
the return value of "floatval" is immediately cased to a "long". The now used
virXPathNumber() returns a "double".
>I like the idea of outputting -1 rather than the unsigned all-ones
>value; which means you need to respin this patch to avoid testsuite
>failures where we change the test output.
See attached patch.
>Also, anywhere that the
>parser doesn't accept -1, we need to fix that; accepting -1 as the only
>valid negative value and requiring all other values to be non-negative
>makes sense.
I had a look at the relax-ng schema: schemas/storagepool.rng accepts the -1,
scheams/storagevol.rng does not.
On the other hand the "-1" seems not to be documented on
(btw: out of date, see 2. patch)
The more I think about the -1 problem, the more I'm getting confused: in
virStorageBackendFileSystemBuild() there is some code which reads back the
actual owner and group, but my /etc/libvirt/storage/default.xml still contains
the -1 / 4294967295, which also seems to be returned by "virsh pool-dumpxml
default".
Personally I would find the following semantics the least confusing:
1. on define, -1 stands for "the current gid / pid" for the future time, when
the pool is started.
2. on create/start, -1 would be my gid/pid for qemu:///session and
root:libvirt (or whatever) for qemu:///system .
3. on read back of an active pool, the current uid/gid are returned, so never
-1. (the target exists, so the current values could be returned; a -1 is not
very useful for the user here, IMHO.)
4. on read back of an passive pool, the original uid/gid (including -1) should
be returned. (the target might not exist, so libvirt can only show the
intended values)
I would like to get that clarified before spending more time on a proper v2,
since I now have to switch to a different task requiring my immediate
attention.
>> - Some regression test?
>
>Did you run 'make check' on your patch? We already have XML output that
>is affected by the change in output format, and can write the test so
>that we prove that multiple ways of formatting input all get
>canonicalized to the same output.
No, forgot to do that.
Thank you for your review and comments. Much appreciated.
Sincerely
Philipp
--
Philipp Hahn Open Source Software Engineer h...@univention.de
Univention GmbHbe open. fon: +49 421 22 232- 0
Mary-Somerville-Str.1 D-28359 Bremen fax: +49 421 22 232-99
http://www.univention.de/
b/tests/storagevolxml2xmlout/vol-sheepdog.xmldiff --git a/docs/storage.html.in b/docs/storage.html.in
index 26a949a..983568c 100644
--- a/docs/storage.html.in
+++ b/docs/storage.html.in
@@ -115,6 +115,7 @@
+
Directory pool
A pool with a type of dir provides the means to manage
@@ -153,6 +154,7 @@
iso: CDROM disk image format
qcow: QEMU v1 disk image format
qcow2: QEMU v2 disk image format
+ qed: QEMU Enhanced Disk disk image form
VIR_ERR_NO_CONNECT already contains "no connection driver available".
This patch changes:
no connection driver available for No connection for URI hello
to:
no connection driver available for hello
Bug: https://bugzilla.redhat.com/show_bug.cgi?id=851413
---
src/libvirt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/libvirt.c b/src/libvirt.c
index 934997a..8a28e4a 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -1216,7 +1216,7 @@ do_open(const char *name,
if (!ret->driver) {
/* If we reach here, then all drivers declined the connection. */
virLibConnError(VIR_ERR_NO_CONNECT,
-_("No connection for URI %s"),
+"%s",
NULLSTR(name));
goto failed;
}
--
1.7.12.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On Mon, Feb 25, 2013 at 12:41:06 +, Daniel P. Berrange wrote:
...
> Then I think we'll duplicate all the APIs for setting resource tunables
> from virDomainPtr against the new object, so we get
>
>
> int virPartitionGetSchedulerParameters(virPartitionPtr partition,
>virTypedParameterPtr params,
>int *nparams,
>unsigned int flags);
My comment is not really specific to resource groups but since you
suggest to copy APIs that return typed parameters, could you make them
autoallocate params array (see virDomainJobGetStats)? That is
int virPartitionGetSchedulerParameters(virPartitionPtr partition,
virTypedParameterPtr *params,
int *nparams,
unsigned int flags);
Having to call each API twice, first to get the number of parameters it
can return and then to get the actual parameters is a horrible design.
Jirka
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Currently, qemuDomainShutdownFlags() chooses the agent method of
shutdown whenever the agent is configured. However, this
assumption is not enough as the guest agent may be unresponsive
at the moment. So unless guest agent method has been explicitly
requested, we should fall back to the ACPI method.
---
diff to v1:
- Rework some conditions as Eric suggested in v1
src/qemu/qemu_driver.c | 38 ++
1 file changed, 22 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1e96915..1e96aa4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1702,40 +1702,40 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags) {
virDomainObjPtr vm;
int ret = -1;
qemuDomainObjPrivatePtr priv;
-bool useAgent = false;
+bool useAgent = false, agentRequested, acpiRequested;
virCheckFlags(VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN |
VIR_DOMAIN_SHUTDOWN_GUEST_AGENT, -1);
-/* At most one of these two flags should be set. */
-if ((flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) &&
-(flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT)) {
-virReportInvalidArg(flags, "%s",
-_("flags for acpi power button and guest agent are
mutually exclusive"));
-return -1;
-}
-
if (!(vm = qemuDomObjFromDomain(dom)))
goto cleanup;
priv = vm->privateData;
+agentRequested = flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT;
+acpiRequested = flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN;
-if ((flags & VIR_DOMAIN_SHUTDOWN_GUEST_AGENT) ||
-(!(flags & VIR_DOMAIN_SHUTDOWN_ACPI_POWER_BTN) &&
- priv->agent))
+/* Prefer agent unless we were requested to not to. */
+if (agentRequested || (!flags && priv->agent))
useAgent = true;
-if (useAgent) {
-if (priv->agentError) {
+if (priv->agentError) {
+if (agentRequested && !acpiRequested) {
virReportError(VIR_ERR_AGENT_UNRESPONSIVE, "%s",
_("QEMU guest agent is not "
"available due to an error"));
goto cleanup;
+} else {
+useAgent = false;
}
-if (!priv->agent) {
+}
+
+if (!priv->agent) {
+if (agentRequested && !acpiRequested) {
virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
_("QEMU guest agent is not configured"));
goto cleanup;
+} else {
+useAgent = false;
}
}
@@ -1752,7 +1752,13 @@ static int qemuDomainShutdownFlags(virDomainPtr dom,
unsigned int flags) {
qemuDomainObjEnterAgent(vm);
ret = qemuAgentShutdown(priv->agent, QEMU_AGENT_SHUTDOWN_POWERDOWN);
qemuDomainObjExitAgent(vm);
-} else {
+}
+
+/* If we are not enforced to use just an agent, try ACPI
+ * shutdown as well in case agent did not succeed.
+ */
+if (!useAgent ||
+(ret < 0 && (acpiRequested || !flags))) {
qemuDomainSetFakeReboot(driver, vm, false);
qemuDomainObjEnterMonitor(driver, vm);
--
1.8.1.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Sorry, I only noticed your reply today.
On 02/19/13 17:36, John Ferlan wrote:
> On 02/15/2013 05:13 AM, Peter Krempa wrote:
>> On 02/15/13 11:00, Ján Tomko wrote:
>>> Use virURIParse in qemuMigrationPrepareDirect to allow parsing
>>> IPv6 addresses, which would cause an 'incorrect :port' error message
>>> before.
>>>
>>> To be able to migrate over IPv6, QEMU needs to listen on [::] instead
>>> of 0.0.0.0. This patch adds a call to getaddrinfo and sets the listen
>>> address based on the result.
>>>
>>> This will break migration if a hostname that can only be resolved on the
>>> source machine is passed in the migration URI, or if it does not resolve
>>> to the same address family on both sides.
>
> Yuck... "feature-wise" you can pass hints to getaddrinfo() to tell it
> what type of address family you want... You can walk the info->ai_next
> list looking for the family you want/prefer. Secondary feature-wise
> qemu could say which it prefers to use on the "-incoming" value...
In this case, I assume the first family returned by getaddrinfo is the
preferred one. I'm not sure how qemu would have more clue than libvirt
about which address to use.
>>> }
>>> }
>>>
>>> +if (getaddrinfo(hostname, NULL, NULL, &info)) {
>>> +virReportError(VIR_ERR_INTERNAL_ERROR,
>>> + _("unable to get address info for %s"),
>>> hostname);
>>> +goto cleanup;
>>
>> Isn't there a possibility to fall back on IPv4 if this fails to minimize
>> the chance of regressing?
>>
>
> Yeah - this is tricky for some of the reasons I listed above. It seems
> we need a way to tell or force the target qemu to use one family style
> over the other because that's what the source resolved to using.
Qemu does support ipv4 or ipv6 flags for hostnames. From a quick glance
at the git history it seems it always has. Maybe we could always add the
address family option to the migration URI to force this?
>
> Assuming I'm reading things correctly we are about to tell the target
> qemu how to start and to listen over the "first" style of address we
> found in the source hosts' list. The source connect will use that
> uri_out to perform the migration. The issue I can see becomes what if
> the target (for some reason) doesn't support/have/use the family that
> the host found first? When it goes to start up a localhost port, it
> will fail right? Then what?
>
No, the perform phase happens on the destination, but the result is
still the same. If the families do not match (or they do match but they
can't connect to each other via that family), migration will fail. Then
the user either has to change the network configuration or specify the
IP address directly.
>>> if (*uri_out)
>>> VIR_DEBUG("Generated uri_out=%s", *uri_out);
>>>
>>> -/* QEMU will be started with -incoming tcp:0.0.0.0:port */
>>> -snprintf(migrateFrom, sizeof(migrateFrom), "tcp:0.0.0.0:%d",
>>> this_port);
>>> +/* QEMU will be started with -incoming tcp:0.0.0.0:port
>>> + * or -incoming tcp:[::]:port for IPv6 */
>>> +if (ipv6) {
>>> +snprintf(migrateFrom, sizeof(migrateFrom),
>>> + "tcp:[::]:%d", this_port);
>
> I thought IPv6 localhost was "::1"... Or is "::" a synonym? It's been a
> while since I had to think about this and just took a quick google...
>
It's the IPv6 equivalent of 0.0.0.0, meaning any address.
We don't allow migration on localhost.
>>> +} else {
>>> +snprintf(migrateFrom, sizeof(migrateFrom),
>>> + "tcp:0.0.0.0:%d", this_port);
>>
>> I thing this would be doable. Just do IPv4 by default if the resolution
>> fails.
>>
>
> Hmm... which resolution fails do you mean? Perhaps the "feature" needs
> to be set/check some field that says the target qemu wants/desires IPv6;
> otherwise, always fall back to use IPv4.
>
If the hostname specified by the user can only be resolved on the
source, migration still works, but erroring out on getaddrinfo failure
would break it.
We can definitely tell qemu to listen on v4/v6 if we received an IP
address. As for hostnames, we can either guess it from how it resolves
on the source, destination or get the input from the user. Maybe we
could add a migration flag for this and add ipv4 or ipv6 option to the
migration URI for qemu based on the presence/absence of this flag. Or
only do IPv6 migration if a URI with a v6 address or tcp6: scheme was
present?
Jan
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Eduardo Habkost writes:
> This allows ":" to be used a separator between each CPU range, so the
> command-line may look like:
>
> -numa node,cpus=A-B:C-D
>
> Note that the following format, currently used by libvirt:
>
> -numa nodes,cpus=A-B,C-D
>
> will _not_ work, as "," is the option separator for the command-line
> option parser, and it would require changing the -numa option parsing
> code to handle "cpus" as a special case.
>
> Signed-off-by: Eduardo Habkost
> ---
> Changes v2:
> - Use ":" as separator
> - Document the new format
See also discussion on multi-valued keys in command line option
arguments and config files in v1 thread. Hopefully we can reach a
conclusion soon, and then we'll see whether this patch is what we want.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 02/25/2013 12:52 PM, Daniel Veillard wrote:
I have just tagged git and pushed the tarball. The rpms for F17
are following too at the usual place:
ftp://libvirt.org/libvirt/
I gave a try to the set of rpms and this seems to work fine for
relatively simple tests, but of course more testing and checking on
other architectures is needed !
If everything goes fine, I will make an rc2 in a couple of days
and then we can decide if a final release on Friday is fine or if
we need to postpone to next week,
Daniel
Can we please revert the following commits before rc2;
24aa7f8d11054b7b2e643cf3cd5c80a199764af0 (S390: Documentation for CCW
address type)
0bbbd42c30543d8251536c2fa11166834c886ada (S390: domain_conf support for CCW)
this is for two reasons:
1. They don't make sense without the rest of the series which is still
pending review (and probably needs a rebase meanwhile) and the result
already shows up on libvirt.org
2. I received feedback from someone more platform-savvy than I am, that
one of the XML attribute names (schid) is misleading
Reverting gives me the opportunity to provide a new series on top of
1.0.3 which can be hopefully applied in time for the next release.
--
Mit freundlichen Grüßen/Kind Regards
Viktor Mihajlovski
IBM Deutschland Research & Development GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter
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
Eduardo Habkost writes:
> On Thu, Feb 21, 2013 at 09:23:22PM +0100, Markus Armbruster wrote:
>> Eduardo Habkost writes:
>>
>> > This allows "," to be used a separator between each CPU range. Note
>> > that commas inside key=value command-line options have to be escaped
>> > using ",,", so the command-line will look like:
>> >
>> > -numa node,cpus=A,,B,,C,,D
>>
>> This is really, really ugly, and an embarrassment to document. Which
>> you didn't ;)
>
> I was trying to have an intermediate solution using the current -numa
> parser. I have patches in my queue that will change the code to properly
> use QemuOpts later.
>
> It would be interesting to support the "A,B,C,D" format in config files,
> though, as it is simple and straighforward when no escaping is required.
Our config file syntax is in a Windows INI dialect: key=value lines
grouped into sections. Our dialect requires values to be enclosed in
quotes. Commonly, the quotes are optional. Could be fixed. It
supports multi-valued keys the common INI way: multiple key=value lines
for the same key, one per value
key = "A,B,C" works when the A, B, C can't contain commas. Fine for a
list of numbers. For long lists, we'd probably want to add a line
continuation feature.
Strings can contain commas, so you'd have to do something like key =
"A", "B", "C". Whether that's still Windows INI is debatable. More so
since there's already a common way to do it: one line per value.
If we decide INI doesn't meet our needs or desires for pretty syntax, we
should not extend it beyond its limits into QEMU's very own
configuration syntax. We should switch to a common syntax that serves
our needs and desires. For what it's worth, we already parse JSON.
For me, the INI way to do multi-valued keys is still fine.
>> What about
>>
>> -numa node,cpus=A,cpus=B,cpus=C,cpus=D
>
> Looks better for the command-line usage, at least. I will give it a try.
>
>>
>> Yes, QemuOpts lets you do that. Getting all the values isn't as easy as
>> it could be (unless you use Laszlo's opt-visitor), but that could be
>> improved.
>
> Guess what: -numa doesn't even use QemuOpts, and I am not sure the
> current format of -numa will allow QemuOpts to be used easily. I expect
> the proper solution using QemuOpts to involve having a
> standards-compliant "numa-node" config section instead of this weird
> "-numa ,..." format where the only valid that ever existed
> was "node".
This is the current -numa syntax, as far as I can tell:
-numa node,KEY=VALUE,...
Recognized KEY=VALUE:
nodeid=UINT
mem=SIZE
cpus=[|UINT|UINT-UINT]
Unrecognized KEYs are silently ignored.
This should fit into QemuOpts just fine. Sketch:
static QemuOptsList qemu_numa_opts = {
.name = "numa",
.implied_opt_name = "type"
.head = QTAILQ_HEAD_INITIALIZER(qemu_rtc_numa.head),
.desc = {
{
.name = "type",
.type = QEMU_OPT_STRING,
.help = "node type"
}, {
.name = "nodeid",
.type = QEMU_OPT_NUMBER,
.help = "node ID"
}, {
.name = "mem",
.type = QEMU_OPT_SIZE,
.help = "memory size"
}, {
.name = "cpus",
.type = QEMU_OPT_STRING,
.help = "CPU range"
},
{ /* end of list */ }
},
};
type = qemu_opt_get(opts);
if (!type || strcmp(type, "node)) {
// error
}
// get and record nodeid, mem
// get, parse and record cpus
This rejects unrecognized keys, unlike the current code. Declare bug
fix ;)
To support discontinuous CPU sets, simply get all values of key "cpus".
> But I believe it will be feasible to allow "cpus=A,cpus=B" using the
> current parser, before we convert to a proper QemuOpts-based
> implementaiton.
>
>>
>> > Note that the following format, currently used by libvirt:
>> >
>> > -numa nodes,cpus=A,B,C,D
>> >
>> > will _not_ work yet, as "," is the option separator for the command-line
>> > option parser, and it will require changing the -numa option parsing
>> > code to handle "cpus" as a special case.
>>
>> No way.
>
> Agreed. :-)
>
> The bad news is that libvirt uses this format since forever, this format
> never worked, and nobody ever noticed that this was broken.
The good news is that it never worked, which simplifies our backward
compatibility worries.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
