Re: [libvirt] [libvirt-glib] Keep domain devices list sorted
On Wed, Mar 07, 2012 at 12:27:53AM +0200, Zeeshan Ali (Khattak) wrote: From: Zeeshan Ali (Khattak) zeesha...@gnome.org While we don't guarantee the order of devices in the list returned from gvir_domain_get_devices(), its not a bad idea to keep them sorted the same way we get them from configuration (XML). --- libvirt-gobject/libvirt-gobject-domain.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/libvirt-gobject/libvirt-gobject-domain.c b/libvirt-gobject/libvirt-gobject-domain.c index 7b2c7ad..0bafa7e 100644 --- a/libvirt-gobject/libvirt-gobject-domain.c +++ b/libvirt-gobject/libvirt-gobject-domain.c @@ -909,5 +909,5 @@ GList *gvir_domain_get_devices(GVirDomain *domain, } g_list_free (config_devices); -return ret; +return g_list_reverse (ret); } ACK Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3] virsh: add option aliases
On 03/07/2012 03:33 PM, Osier Yang wrote:
On 03/03/2012 09:02 AM, Eric Blake wrote:
In the past, we have created some virsh options with less-than-stellar
names. For back-compat reasons, those names must continue to parse,
but we don't want to document them in help output. This introduces
a new option type, an alias, which points to a canonical option name
later in the option list.
I'm actually quite impressed that our code has already been factored
to do all option parsing through common entry points, such that I
got this added in relatively few lines of code!
* tools/virsh.c (VSH_OT_ALIAS): New option type.
(opts_echo): Hook up an alias, for easy testing.
(vshCmddefOptParse, vshCmddefHelp, vshCmddefGetOption): Allow for
aliases.
* tests/virshtest.c (mymain): Test new feature.
---
tests/virshtest.c | 6 ++
tools/virsh.c | 28 ++--
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/tests/virshtest.c b/tests/virshtest.c
index 6474c19..87b1336 100644
--- a/tests/virshtest.c
+++ b/tests/virshtest.c
@@ -386,6 +386,12 @@ mymain(void)
DO_TEST(30, --shell a\n,
echo \t '-'\-\ \t --shell \t a);
+ /* Tests of alias handling. */
+ DO_TEST(31, hello\n, echo, --string, hello);
+ DO_TEST(32, hello\n, echo --string hello);
+ DO_TEST(33, hello\n, echo, --str, hello);
+ DO_TEST(34, hello\n, echo --str hello);
+
# undef DO_TEST
VIR_FREE(custom_uri);
diff --git a/tools/virsh.c b/tools/virsh.c
index aef050f..77cf4ac 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -138,7 +138,8 @@ typedef enum {
VSH_OT_STRING, /* optional string option */
VSH_OT_INT, /* optional or mandatory int option */
VSH_OT_DATA, /* string data (as non-option) */
- VSH_OT_ARGV /* remaining arguments */
+ VSH_OT_ARGV, /* remaining arguments */
+ VSH_OT_ALIAS, /* alternate spelling for a later argument */
} vshCmdOptType;
/*
@@ -190,7 +191,8 @@ typedef struct {
const char *name; /* the name of option, or NULL for list end */
vshCmdOptType type; /* option type */
unsigned int flags; /* flags */
- const char *help; /* non-NULL help string */
+ const char *help; /* non-NULL help string; or for VSH_OT_ALIAS
+ * the name of a later public option */
} vshCmdOptDef;
/*
@@ -15209,6 +15211,7 @@ static const vshCmdInfo info_echo[] = {
static const vshCmdOptDef opts_echo[] = {
{shell, VSH_OT_BOOL, 0, N_(escape for shell use)},
{xml, VSH_OT_BOOL, 0, N_(escape for XML use)},
+ {str, VSH_OT_ALIAS, 0, string},
{string, VSH_OT_ARGV, 0, N_(arguments to echo)},
{NULL, 0, 0, NULL}
};
@@ -17256,6 +17259,18 @@ vshCmddefOptParse(const vshCmdDef *cmd,
uint32_t *opts_need_arg,
return -1; /* bool options can't be mandatory */
continue;
}
+ if (opt-type == VSH_OT_ALIAS) {
+ int j;
+ if (opt-flags || !opt-help)
+ return -1; /* alias options are tracked by the original name */
+ for (j = i + 1; cmd-opts[j].name; j++) {
+ if (STREQ(opt-help, cmd-opts[j].name))
+ break;
+ }
+ if (!cmd-opts[j].name)
+ return -1; /* alias option must map to a later option name */
+ continue;
+ }
if (opt-flags VSH_OFLAG_REQ_OPT) {
if (opt-flags VSH_OFLAG_REQ)
*opts_required |= 1 i;
@@ -17287,6 +17302,10 @@ vshCmddefGetOption(vshControl *ctl, const
vshCmdDef *cmd, const char *name,
const vshCmdOptDef *opt =cmd-opts[i];
if (STREQ(opt-name, name)) {
+ if (opt-type == VSH_OT_ALIAS) {
+ name = opt-help;
+ continue;
+ }
if ((*opts_seen (1 i)) opt-type != VSH_OT_ARGV) {
vshError(ctl, _(option --%s already seen), name);
return NULL;
@@ -17465,6 +17484,9 @@ vshCmddefHelp(vshControl *ctl, const char
*cmdname)
: _([%s]...);
}
break;
+ case VSH_OT_ALIAS:
+ /* aliases are intentionally undocumented */
+ continue;
default:
assert(0);
}
@@ -17506,6 +17528,8 @@ vshCmddefHelp(vshControl *ctl, const char
*cmdname)
shortopt ? _([--%s]string) : _(%s),
opt-name);
break;
+ case VSH_OT_ALIAS:
+ continue;
default:
assert(0);
}
Ah, I could recall we talked about this half year before, for
creating alias (--config) for the options --persistent of
commands like attach-device, then I forgot it to do it.
I created a patch which can be served as 4/4 of these series,
for the --persistent changes.
Osier
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] virsh: Use option alias for outmoded --persistent
Since VIR_DOMAIN_AFFECT_{LIVE,CONFIG,CURRENT} was created,
all new virsh commands use --config to represents the
persistent changing. This patch add --config option
for the old commands which still use --persistent,
and --persistent is now alias of --config.
tools/virsh.c: (use --config, and --persistent is
alias of --config now).
cmdDomIfSetLink, cmdDomIfGetLink, cmdAttachDevice,
cmdDetachDevice, cmdUpdateDevice, cmdAttachInterface,
cmdDetachInterface, cmdAttachDisk, cmdDetachDisk
toos/virsh.pod: Update docs of the changed commands, and
add some missed docs for --config (detach-interface,
detach-disk, and detach-device).
---
tools/virsh.c | 51 ++-
tools/virsh.pod | 36 ++--
2 files changed, 52 insertions(+), 35 deletions(-)
diff --git a/tools/virsh.c b/tools/virsh.c
index ba3ea1c..3535ad1 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -1635,7 +1635,8 @@ static const vshCmdOptDef opts_domif_setlink[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
{interface, VSH_OT_DATA, VSH_OFLAG_REQ, N_(interface device (MAC
Address))},
{state, VSH_OT_DATA, VSH_OFLAG_REQ, N_(new state of the device)},
-{persistent, VSH_OT_BOOL, 0, N_(persist interface state)},
+{persistent, VSH_OT_ALIAS, 0, config},
+{config, VSH_OT_BOOL, 0, N_(affect next boot)},
{NULL, 0, 0, NULL}
};
@@ -1650,7 +1651,7 @@ cmdDomIfSetLink (vshControl *ctl, const vshCmd *cmd)
unsigned char macaddr[VIR_MAC_BUFLEN];
const char *element;
const char *attr;
-bool persistent;
+bool config;
bool ret = false;
unsigned int flags = 0;
int i;
@@ -1672,7 +1673,7 @@ cmdDomIfSetLink (vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptString(cmd, state, state) = 0)
goto cleanup;
-persistent = vshCommandOptBool(cmd, persistent);
+config = vshCommandOptBool(cmd, config);
if (STRNEQ(state, up) STRNEQ(state, down)) {
vshError(ctl, _(invalid link state '%s'), state);
@@ -1680,13 +1681,13 @@ cmdDomIfSetLink (vshControl *ctl, const vshCmd *cmd)
}
/* get persistent or live description of network device */
-desc = virDomainGetXMLDesc(dom, persistent?VIR_DOMAIN_XML_INACTIVE:0);
+desc = virDomainGetXMLDesc(dom, config ? VIR_DOMAIN_XML_INACTIVE : 0);
if (desc == NULL) {
vshError(ctl, _(Failed to get domain description xml));
goto cleanup;
}
-if (persistent)
+if (config)
flags = VIR_DOMAIN_AFFECT_CONFIG;
else
flags = VIR_DOMAIN_AFFECT_LIVE;
@@ -1810,7 +1811,8 @@ static const vshCmdInfo info_domif_getlink[] = {
static const vshCmdOptDef opts_domif_getlink[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
{interface, VSH_OT_DATA, VSH_OFLAG_REQ, N_(interface device (MAC
Address))},
-{persistent, VSH_OT_BOOL, 0, N_(Get persistent interface state)},
+{persistent, VSH_OT_ALIAS, 0, config},
+{config, VSH_OT_BOOL, 0, N_(Get persistent interface state)},
{NULL, 0, 0, NULL}
};
@@ -1844,7 +1846,7 @@ cmdDomIfGetLink (vshControl *ctl, const vshCmd *cmd)
return false;
}
-if (vshCommandOptBool(cmd, persistent))
+if (vshCommandOptBool(cmd, config))
flags = VIR_DOMAIN_XML_INACTIVE;
desc = virDomainGetXMLDesc(dom, flags);
@@ -13439,7 +13441,8 @@ static const vshCmdInfo info_attach_device[] = {
static const vshCmdOptDef opts_attach_device[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
{file, VSH_OT_DATA, VSH_OFLAG_REQ, N_(XML file)},
-{persistent, VSH_OT_BOOL, 0, N_(persist device attachment)},
+{persistent, VSH_OT_ALIAS, 0, config},
+{config, VSH_OT_BOOL, 0, N_(affect next boot)},
{NULL, 0, 0, NULL}
};
@@ -13469,7 +13472,7 @@ cmdAttachDevice(vshControl *ctl, const vshCmd *cmd)
return false;
}
-if (vshCommandOptBool(cmd, persistent)) {
+if (vshCommandOptBool(cmd, config)) {
flags = VIR_DOMAIN_AFFECT_CONFIG;
if (virDomainIsActive(dom) == 1)
flags |= VIR_DOMAIN_AFFECT_LIVE;
@@ -13750,7 +13753,8 @@ static const vshCmdInfo info_detach_device[] = {
static const vshCmdOptDef opts_detach_device[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
{file, VSH_OT_DATA, VSH_OFLAG_REQ, N_(XML file)},
-{persistent, VSH_OT_BOOL, 0, N_(persist device detachment)},
+{persistent, VSH_OT_ALIAS, 0, config},
+{config, VSH_OT_BOOL, 0, N_(affect next boot)},
{NULL, 0, 0, NULL}
};
@@ -13778,7 +13782,7 @@ cmdDetachDevice(vshControl *ctl, const vshCmd *cmd)
goto cleanup;
}
-if (vshCommandOptBool(cmd, persistent)) {
+if (vshCommandOptBool(cmd, config)) {
flags = VIR_DOMAIN_AFFECT_CONFIG;
if (virDomainIsActive(dom) == 1)
flags |= VIR_DOMAIN_AFFECT_LIVE;
@@ -13814,7 +13818,8 @@ static
Re: [libvirt] [PATCHv2 13/15] virsh: add command aliases, and rename nodedev-detach
On 03/06/2012 01:34 AM, Eric Blake wrote: Just because our public API has a typo doesn't mean that virsh has to keep the typo. * tools/virsh.c (VSH_CMD_FLAG_ALIAS): New flag. (nodedevCmds): Use it. (cmdHelp): Omit alias commands. (cmdNodeDeviceDettach): Rename... (cmdNodeDeviceDetach): ...to this. * tools/virsh.pod (nodedev-detach): Document it. --- Osier ACKed this in his review of the previous posting. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3 V7] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.
On Tue, Mar 06, 2012 at 10:02:15PM -0700, Eric Blake wrote: On 03/06/2012 05:10 AM, Richard W.M. Jones wrote: This all appears to work. I built my own libvirt with the three remaining patches and was able to read pCPU information for a running domain. The libvirt side is now pushed. Rich: The documentation in libvirt.c correctly says that calling virDomainGetCPUStats(dom, NULL, 0, 0, 0, 0) must tell you part of the information needed to compute the size of an array to allocate; but in v7, the return was off by one too small. But it was generally masked by the v7 virsh code overallocating to a fixed 128 slots rather than paying attention to the return value. I fixed both those problems before pushing the libvirt patches, but since you tested with v7 instead of my fixed version, you may need to double check that virt-top isn't making the same mistakes, as it might be a boundary case that only strikes on machines with 128 physical CPUs. TBH I found the documentation for virDomainGetCPUStats to be very confusing indeed. I couldn't really tell if virt-top is calling the API correctly or not, so I simply used Fujitsu's code directly. Do you have any comments on whether this is correct or not? http://git.annexia.org/?p=ocaml-libvirt.git;a=blob;f=libvirt/libvirt_c_oneoffs.c;h=f827707a77e6478129370fce67e46ae745b9be9a;hb=HEAD#l534 Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones New in Fedora 11: Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 70 libraries supprt'd http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] ANNOUNCE: libvirt-glib release 0.0.6
I am pleased to announce that a new release of the libvirt-glib package, version 0.0.6 is now available from ftp://libvirt.org/libvirt/glib/ The packages are GPG signed with Key fingerprint: DAF3 A6FD B26B 6291 2D0E 8E3F BE86 EBB4 1510 4FDF (4096R) New in this release: - Add binding for virDomainBlockResize(): gvir_domain_disk_resize(). - Set correct target node attribute for domain interface. gvir_config_domain_interface_set_ifname() should be setting 'dev' attribute under 'target', not 'device'. - Getter for the associated domain of a domain device. - Getters for GVirConfigDomainInterface attributes. - GVirDomainDevice now has an associated GVirConfigDomainDevice. - Remove now redundant 'path' property from GVirDomainDevice subclasses. - Add gvir_domain_get_devices(). - Empty statistics for user-mode interfaces. One of the limitations of user-mode networking of libvirt is that you can't get statistics for it (not yet, at least). Instead of erroring-out in that case, simply return empty statistics result and spit a debug message. - Fix a GVirStream leak. - Also distribute GNUmakefile, cfg.mk and maint.mk files. libvirt-glib comprises three distinct libraries: - libvirt-glib- Integrate with the GLib event loop and error handling - libvirt-gconfig - Representation of libvirt XML documents as GObjects - libvirt-gobject - Mapping of libvirt APIs into the GObject type system NB: While libvirt aims to be API/ABI stable, for the first few releases, we are *NOT* guaranteeing that libvirt-glib libraries are API/ABI stable. ABI stability will only be guaranteed once the bulk of the APIs have been fleshed out and proved in non-trivial application usage. We anticipate this will be within the next few months in order to line up with Fedora 17. Follow up comments about libvirt-glib should be directed to the regular libvir-list redhat com development list. Thanks to all the people involved in contributing to this release. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH libvirt-glib] Fix generation of docs in a VPATH builder
From: Daniel P. Berrange berra...@redhat.com The DOC_SOURCE_DIR variable was missing the $(top_srcdir) variable so it could not find the source files when run from a VPATH build. Empirically the previous comment saying that $(top_srcdir) was not needed is wrong. --- docs/libvirt-gconfig/Makefile.am |4 +--- docs/libvirt-glib/Makefile.am|4 +--- docs/libvirt-gobject/Makefile.am |4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/docs/libvirt-gconfig/Makefile.am b/docs/libvirt-gconfig/Makefile.am index 2af0d90..6a3df37 100644 --- a/docs/libvirt-gconfig/Makefile.am +++ b/docs/libvirt-gconfig/Makefile.am @@ -3,9 +3,7 @@ DOC_MODULE=Libvirt-gconfig DOC_MAIN_SGML_FILE=$(DOC_MODULE)-docs.xml -# Must not use $(top_srcdir) since gtkdoc-scan runs -# from the srcdir already, not the builddir -DOC_SOURCE_DIR=../../libvirt-gconfig +DOC_SOURCE_DIR=$(top_srcdir)/libvirt-gconfig SCANGOBJ_OPTIONS= diff --git a/docs/libvirt-glib/Makefile.am b/docs/libvirt-glib/Makefile.am index 81e9671..e45f4e2 100644 --- a/docs/libvirt-glib/Makefile.am +++ b/docs/libvirt-glib/Makefile.am @@ -3,9 +3,7 @@ DOC_MODULE=Libvirt-glib DOC_MAIN_SGML_FILE=$(DOC_MODULE)-docs.xml -# Must not use $(top_srcdir) since gtkdoc-scan runs -# from the srcdir already, not the builddir -DOC_SOURCE_DIR=../../libvirt-glib +DOC_SOURCE_DIR=$(top_srcdir)/libvirt-glib SCANGOBJ_OPTIONS= diff --git a/docs/libvirt-gobject/Makefile.am b/docs/libvirt-gobject/Makefile.am index 6eb3553..c068a96 100644 --- a/docs/libvirt-gobject/Makefile.am +++ b/docs/libvirt-gobject/Makefile.am @@ -3,9 +3,7 @@ DOC_MODULE=Libvirt-gobject DOC_MAIN_SGML_FILE=$(DOC_MODULE)-docs.xml -# Must not use $(top_srcdir) since gtkdoc-scan runs -# from the srcdir already, not the builddir -DOC_SOURCE_DIR=../../libvirt-gobject +DOC_SOURCE_DIR=$(top_srcdir)/libvirt-gobject SCANGOBJ_OPTIONS= -- 1.7.7.6 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Ensure max_id is initialized in linuxParseCPUmap()
From: Daniel P. Berrange berra...@redhat.com
Pushing as a build-breaker fix
---
src/nodeinfo.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/nodeinfo.c b/src/nodeinfo.c
index 709e94a..61a5925 100644
--- a/src/nodeinfo.c
+++ b/src/nodeinfo.c
@@ -581,7 +581,7 @@ linuxParseCPUmap(int *max_cpuid, const char *path)
{
char *map = NULL;
char *str = NULL;
-int max_id, i;
+int max_id = 0, i;
if (virFileReadAll(path, 5 * VIR_DOMAIN_CPUMASK_LEN, str) 0) {
virReportOOMError();
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Patch]: spice agent-mouse support [v3]
Sorry my git send-email failed to the list :-(
Signed-off-by: Zhou Peng zhoup...@nfs.iscas.ac.cn
spice agent-mouse support
Usage:
graphics type='spice'
mouse mode='client'|'server'/
graphics/
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
index 6fcca94..b63f6a0 100644
--- a/docs/formatdomain.html.in
+++ b/docs/formatdomain.html.in
@@ -2809,6 +2809,13 @@ qemu-kvm -net nic,model=? /dev/null
to codeno/code, span class=sincesince
0.9.3/span.
/p
+p
+ Mouse mode is set by the codemousecode/ element,
+ setting it's codemodecode/ attribute to one of
+ codeserver/code or codeclient/code ,
+ span class=sincesince 0.9.11/span. If no mode is
+ specified, the spice default will be used (client mode).
+/p
/dd
dtcoderdp/code/dt
dd
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 3908733..bb0df03 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -1776,6 +1776,17 @@
empty/
/element
/optional
+optional
+ element name=mouse
+attribute name=mode
+ choice
+valueserver/value
+valueclient/value
+ /choice
+/attribute
+empty/
+ /element
+/optional
/interleave
/group
group
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f9654f1..aa31fe6 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -460,6 +460,12 @@ VIR_ENUM_IMPL(virDomainGraphicsSpicePlaybackCompression,
on,
off);
+VIR_ENUM_IMPL(virDomainGraphicsSpiceMouseMode,
+ VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_LAST,
+ default,
+ server,
+ client);
+
VIR_ENUM_IMPL(virDomainGraphicsSpiceStreamingMode,
VIR_DOMAIN_GRAPHICS_SPICE_STREAMING_MODE_LAST,
default,
@@ -5710,6 +5716,26 @@ virDomainGraphicsDefParseXML(xmlNodePtr node,
VIR_FREE(copypaste);
def-data.spice.copypaste = copypasteVal;
+} else if (xmlStrEqual(cur-name, BAD_CAST mouse)) {
+const char *mode = virXMLPropString(cur, mode);
+int modeVal;
+
+if (!mode) {
+virDomainReportError(VIR_ERR_XML_ERROR, %s,
+ _(spice mouse missing mode));
+goto error;
+}
+
+if ((modeVal =
virDomainGraphicsSpiceMouseModeTypeFromString(mode)) = 0) {
+virDomainReportError(VIR_ERR_XML_ERROR,
+ _(unknown mouse mode
value '%s'),
+ mode);
+VIR_FREE(mode);
+goto error;
+}
+VIR_FREE(mode);
+
+def-data.spice.mousemode = modeVal;
}
}
cur = cur-next;
@@ -11401,7 +11427,8 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
}
if (!children (def-data.spice.image || def-data.spice.jpeg ||
def-data.spice.zlib || def-data.spice.playback ||
- def-data.spice.streaming ||
def-data.spice.copypaste)) {
+ def-data.spice.streaming ||
def-data.spice.copypaste ||
+ def-data.spice.mousemode)) {
virBufferAddLit(buf, \n);
children = 1;
}
@@ -11420,6 +11447,9 @@ virDomainGraphicsDefFormat(virBufferPtr buf,
if (def-data.spice.streaming)
virBufferAsprintf(buf, streaming mode='%s'/\n,
virDomainGraphicsSpiceStreamingModeTypeToString(def-data.spice.streaming));
+if (def-data.spice.mousemode)
+virBufferAsprintf(buf, mouse mode='%s'/\n,
+
virDomainGraphicsSpiceMouseModeTypeToString(def-data.spice.mousemode));
if (def-data.spice.copypaste)
virBufferAsprintf(buf, clipboard copypaste='%s'/\n,
virDomainGraphicsSpiceClipboardCopypasteTypeToString(def-data.spice.copypaste));
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 596be4d..a9c118a 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1003,6 +1003,14 @@ enum virDomainGraphicsSpicePlaybackCompression {
VIR_DOMAIN_GRAPHICS_SPICE_PLAYBACK_COMPRESSION_LAST
};
+enum virDomainGraphicsSpiceMouseMode {
+VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_DEFAULT = 0,
+VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_SERVER,
+VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_CLIENT,
+
+VIR_DOMAIN_GRAPHICS_SPICE_MOUSE_MODE_LAST
+};
+
enum
Re: [libvirt] [PATCHv2 14/15] virsh: improve storage unit parsing
On 03/06/2012 01:34 AM, Eric Blake wrote:
Now can now do:
virsh vol-resize $vol 10M
virsh blockresize $dom $vol 10M
to get both interfaces to resize to 10MiB. The remaining wart
is that vol-resize defaults to bytes, but blockresize defaults
to KiB, but we can't break existing scripts; oh well, it's no
worse than the same wart of the underlying virDomainBlockResize.
* tools/virsh.c (vshCommandOptScaledInt): New function.
(cmdVolResize): Don't pass negative size.
(cmdVolSize): Use new helper routine.
(cmdBlockResize): Likewise; also support bytes.
* tools/virsh.pod (NOTES): Document suffixes.
(blockresize, vol-create-as, vol-resize): Point to notes.
---
static bool
@@ -11754,7 +11743,7 @@ static const vshCmdInfo info_vol_resize[] = {
static const vshCmdOptDef opts_vol_resize[] = {
{vol, VSH_OT_DATA, VSH_OFLAG_REQ, N_(vol name, key or path)},
{capacity, VSH_OT_DATA, VSH_OFLAG_REQ,
- N_(new capacity for the vol with optional k,M,G,T suffix)},
+ N_(new capacity for the vol, as scaled integer (default bytes))},
{pool, VSH_OT_STRING, 0, N_(pool name or uuid)},
{allocate, VSH_OT_BOOL, 0,
N_(allocate the new capacity, rather than leaving it sparse)},
@@ -11792,16 +11781,12 @@ cmdVolResize(vshControl *ctl, const vshCmd *cmd)
if (vshCommandOptString(cmd, capacity,capacityStr)= 0)
goto cleanup;
if (delta *capacityStr == '-') {
-if (cmdVolSize(capacityStr + 1,capacity) 0) {
-vshError(ctl, _(Malformed size %s), capacityStr);
-goto cleanup;
-}
-capacity = -capacity;
-} else {
-if (cmdVolSize(capacityStr,capacity) 0) {
-vshError(ctl, _(Malformed size %s), capacityStr);
-goto cleanup;
-}
+capacityStr++;
This shift in the string discards the minus sign in error/success
messages, but their meaning remains correct.
+flags |= VIR_STORAGE_VOL_RESIZE_SHRINK;
This change in code ...
+}
+if (cmdVolSize(capacityStr,capacity) 0) {
+vshError(ctl, _(Malformed size %s), capacityStr);
+goto cleanup;
}
if (virStorageVolResize(vol, capacity, flags) == 0) {
@@ -2146,7 +2168,10 @@ is in. Ivol-name-or-key-or-path is the name or key or
path of the volume
to resize. The new capacity might be sparse unless I--allocate is
specified. Normally, Icapacity is the new size, but if I--delta
is present, then it is added to the existing size. Attempts to shrink
-the volume will fail unless I--shrink is present.
+the volume will fail unless I--shrink is present.capacity is
... contradicts with this statment in the virsh man page. With your
change, if the delta is negative the shrink flag for the api call is set
automaticaly.
This was probably added as a safety measure and I'd prefer if we would
require the --shrink flag to mark that the user is sure of what he's
doing, although it should be obvious enough from the minus sign. (I
honestly would prefer a docs change but I'm afraid someone could get mad
at us if he accidentaly corrupts his images.)
+scaled integer (see BNOTES above), which defaults to bytes if there
+is no suffix. This command is only safe for storage volumes not in
+use by an active guest; see also Bblockresize for live resizing.
=back
ACK, if you add the check for --shrink.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH RFC v2] qemu: Support numad
numad is an user-level daemon that monitors NUMA topology and processes resource consumption to facilitate good NUMA resource alignment of applications/virtual machines to improve performance and minimize cost of remote memory latencies. It provides a pre-placement advisory interface, so significant processes can be pre-bound to nodes with sufficient available resources. More details: http://fedoraproject.org/wiki/Features/numad numad -w ncpus:memory_amount is the advisory interface numad provides currently. This patch add the support by introducing a bool XML element: numatune autonuma/ /numatune If it's specified, the number of vcpus and the current memory amount specified in domain XML will be used for numad command line (numad uses MB for memory amount): numad -w $num_of_vcpus:$current_memory_amount / 1024 The advisory nodeset returned from numad will be used to set domain process CPU affinity then. (e.g. qemuProcessInitCpuAffinity). If the user specifies both CPU affinity policy (e.g. (vcpu cpuset=1-10,^7,^84/vcpu) and XML indicating to use numad for the advisory nodeset, the specified CPU affinity will be ignored. Only QEMU/KVM and LXC drivers support it now. v1 - v2: * Since Bill Gray says it doesn't matter to use the number of vcpus and current memory amount as numad cmd line argument, though from sementics point of view, what numad expects are physical CPU numbers, let's go this way. v2 dropped XML cpu required_cpu='4' required_memory='512000'/, and just a new boolean XML element autonuma. Codes are refactored accordingly. * v1 overrides the cpuset specified by vcpu cpuset='1-10,^7'2/vcpu, v2 doesn't do that, but just silently ignored. * xml2xml test is added --- configure.ac |8 ++ docs/formatdomain.html.in | 11 ++- docs/schemas/domaincommon.rng |5 + src/conf/domain_conf.c | 124 +--- src/conf/domain_conf.h |1 + src/lxc/lxc_controller.c | 86 -- src/qemu/qemu_process.c| 86 -- .../qemuxml2argv-numatune-autonuma.xml | 32 + tests/qemuxml2xmltest.c|1 + 9 files changed, 287 insertions(+), 67 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-numatune-autonuma.xml diff --git a/configure.ac b/configure.ac index c9cdd7b..31f0835 100644 --- a/configure.ac +++ b/configure.ac @@ -1445,6 +1445,14 @@ AM_CONDITIONAL([HAVE_NUMACTL], [test $with_numactl != no]) AC_SUBST([NUMACTL_CFLAGS]) AC_SUBST([NUMACTL_LIBS]) +dnl Do we have numad? +if test $with_qemu = yes; then +AC_PATH_PROG([NUMAD], [numad], [], [/bin:/usr/bin:/usr/local/bin:$PATH]) + +if test -n $NUMAD; then +AC_DEFINE_UNQUOTED([NUMAD],[$NUMAD], [Location or name of the numad program]) +fi +fi dnl pcap lib LIBPCAP_CONFIG=pcap-config diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 42f38d3..22872c8 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -505,6 +505,7 @@ ... lt;numatunegt; lt;memory mode=strict nodeset=1-4,^3/gt; +lt;autonuma/gt; lt;/numatunegt; ... lt;/domaingt; @@ -519,7 +520,7 @@ span class='since'Since 0.9.3/span dtcodememory/code/dt dd -The optional codememory/code element specify how to allocate memory +The optional codememory/code element specifies how to allocate memory for the domain process on a NUMA host. It contains two attributes, attribute codemode/code is either 'interleave', 'strict', or 'preferred', @@ -527,6 +528,14 @@ syntax with attribute codecpuset/code of element codevcpu/code. span class='since'Since 0.9.3/span /dd + dd +The optional codeautonuma/code element indicates pinning the virtual CPUs +to the advisory nodeset returned by querying numad (a system daemon that +monitors NUMA topology and usage). With using this element, the physical CPUs +specified by attribute codecpuset/code (of element codevcpu/code) will +be ignored. +span class='since'Since 0.9.11 (QEMU/KVM and LXC only)/span + /dd /dl diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a905457..9066b40 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -549,6 +549,11 @@ /attribute /element /optional + optional +element name=autonuma + empty/ +/element + /optional /element /optional /interleave diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b994718..89d00ae 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7447,7
Re: [libvirt] [PATCHv2 15/15] virsh: improve memory unit parsing
On 03/06/2012 01:34 AM, Eric Blake wrote:
The last vestige of the inaccurate 'kilobytes' when we meant 1024 is
now gone. And virsh is now useful for setting memory in units other
than KiB.
* tools/virsh.c (cmdSetmem, cmdSetmaxmem): Use new helper routine,
allow passing bogus arguments on to hypervisor to test driver
sanity checking, and fix leak on parse error.
(cmdMemtuneGetSize): New helper.
(cmdMemtune): Use it.
* tools/virsh.pod (setmem, setmaxmem, memtune): Document this.
---
static const vshCmdOptDef opts_memtune[] = {
{domain, VSH_OT_DATA, VSH_OFLAG_REQ, N_(domain name, id or uuid)},
{hard-limit, VSH_OT_INT, VSH_OFLAG_NONE,
- N_(Max memory in kilobytes)},
+ N_(Max memory, as scaled integer (default KiB))},
{soft-limit, VSH_OT_INT, VSH_OFLAG_NONE,
- N_(Memory during contention in kilobytes)},
+ N_(Memory during contention, as scaled integer (default KiB))},
{swap-hard-limit, VSH_OT_INT, VSH_OFLAG_NONE,
- N_(Max memory plus swap in kilobytes)},
+ N_(Max memory plus swap, as scaled integer (default KiB))},
{min-guarantee, VSH_OT_INT, VSH_OFLAG_NONE,
- N_(Min guaranteed memory in kilobytes)},
+ N_(Min guaranteed memory, as scaled integer (default KiB))},
{config, VSH_OT_BOOL, 0, N_(affect next boot)},
{live, VSH_OT_BOOL, 0, N_(affect running domain)},
{current, VSH_OT_BOOL, 0, N_(affect current domain)},
{NULL, 0, 0, NULL}
};
+static int
+cmdMemtuneGetSize(const vshCmd *cmd, const char *name, long long *value)
As this is a helper function rename it please to vshMemtuneGetSize to
avoid confusion with command functions.
+{
+int ret;
+unsigned long long tmp;
+const char *str;
+char *end;
+
+ret = vshCommandOptString(cmd, name,str);
+if (ret= 0)
+return ret;
+if (virStrToLong_ll(str,end, 10, value) 0)
+return -1;
+if (*value 0) {
+*value = VIR_DOMAIN_MEMORY_PARAM_UNLIMITED;
+return 1;
+}
+tmp = *value;
+if (virScaleInteger(tmp, end, 1024, LLONG_MAX) 0)
+return -1;
+*value = VIR_DIV_UP(tmp, 1024);
+return 0;
+}
+
static bool
-cmdMemtune(vshControl * ctl, const vshCmd * cmd)
+cmdMemtune(vshControl *ctl, const vshCmd *cmd)
{
virDomainPtr dom;
long long hard_limit = 0, soft_limit = 0, swap_hard_limit = 0;
ACK with the name change.
Peter
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 11/15] virsh: add option aliases
On 03/06/2012 01:34 AM, Eric Blake wrote: In the past, we have created some virsh options with less-than-stellar names. For back-compat reasons, those names must continue to parse, but we don't want to document them in help output. This introduces a new option type, an alias, which points to a canonical option name later in the option list. I'm actually quite impressed that our code has already been factored to do all option parsing through common entry points, such that I got this added in relatively few lines of code! * tools/virsh.c (VSH_OT_ALIAS): New option type. (opts_echo): Hook up an alias, for easy testing. (vshCmddefOptParse, vshCmddefHelp, vshCmddefGetOption): Allow for aliases. * tests/virshtest.c (mymain): Test new feature. --- As a side note: It would be the best with this change to put a warning into the mang page that options that disappeared from the help/man page are not vanished but are obsolete and the user should use the new spellings but we still support the legacy ones. Peter -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] cpu: Add cpu definition for Intel Sandy Bridge cpu type
This patch adds support for the new tsc-deadline feature flag
and a new model to the supported model list describing the
Intel Sandy Bridge platform.
---
Patches adding the SandyBridge cpu type in qemu are on review and not upstream
yet. Please
don't prioritize this patch. I sent it for review as the chance that nothing
will change is
greater than I'll have to post another version. Qemu upstream review:
http://lists.nongnu.org/archive/html/qemu-devel/2012-03/msg00914.html
(I'll push this only after qemu will add this functionality upstream)
Review help (cp from the patches):
tsc-deadline flag:
static const char *ext_feature_name[] = {
fma, cx16, xtpr, pdcm,
NULL, NULL, dca, sse4.1|sse4_1,
sse4.2|sse4_2, x2apic, movbe, popcnt,
-NULL, aes, xsave, osxsave,
+tsc-deadline, aes, xsave, osxsave,
avx, NULL, NULL, hypervisor,
};
SandyBridge model:
[cpudef]
+ name = SandyBridge
+ level = 0xd
+ vendor = GenuineIntel
+ family = 6
+ model = 42
+ stepping = 1
+ feature_edx = sse2 sse fxsr mmx clflush pse36 pat cmov mca pge mtrr sep
apic cx8 mce pae msr tsc pse de fpu
+ feature_ecx = avx xsave aes tsc-deadline popcnt x2apic sse4.2 sse4.1 cx16
ssse3 pclmulqdq sse3
+ extfeature_edx = i64 rdtscp nx syscall
+ extfeature_ecx = lahf_lm
+ xlevel = 0x800A
+ model_id = Intel Xeon E312xx (Sandy Bridge)
(note that the sse3 flag is in libvirt known as pni and i64 is known as lm)
src/cpu/cpu_map.xml | 45 +
1 files changed, 45 insertions(+), 0 deletions(-)
diff --git a/src/cpu/cpu_map.xml b/src/cpu/cpu_map.xml
index 7ef230e..f79a727 100644
--- a/src/cpu/cpu_map.xml
+++ b/src/cpu/cpu_map.xml
@@ -160,6 +160,9 @@
feature name='popcnt' !-- CPUID_EXT_POPCNT --
cpuid function='0x0001' ecx='0x0080'/
/feature
+feature name='tsc-deadline'
+ cpuid function='0x0001' ecx='0x0100'/
+/feature
feature name='aes'
cpuid function='0x0001' ecx='0x0200'/
/feature
@@ -595,6 +598,48 @@
feature name='aes'/
/model
+model name='SandyBridge'
+ vendor name='Intel'/
+ feature name='aes'/
+ feature name='apic'/
+ feature name='avx'/
+ feature name='clflush'/
+ feature name='cmov'/
+ feature name='cx16'/
+ feature name='cx8'/
+ feature name='de'/
+ feature name='fpu'/
+ feature name='fxsr'/
+ feature name='lahf_lm'/
+ feature name='lm'/
+ feature name='mca'/
+ feature name='mce'/
+ feature name='mmx'/
+ feature name='msr'/
+ feature name='mtrr'/
+ feature name='nx'/
+ feature name='pae'/
+ feature name='pat'/
+ feature name='pclmuldq'/
+ feature name='pge'/
+ feature name='pni'/
+ feature name='popcnt'/
+ feature name='pse'/
+ feature name='pse36'/
+ feature name='rdtscp'/
+ feature name='sep'/
+ feature name='sse'/
+ feature name='sse2'/
+ feature name='sse4.1'/
+ feature name='sse4.2'/
+ feature name='ssse3'/
+ feature name='syscall'/
+ feature name='tsc'/
+ feature name='tsc-deadline'/
+ feature name='x2apic'/
+ feature name='xsave'/
+/model
+
model name='Opteron_G1'
vendor name='AMD'/
feature name='sse2'/
--
1.7.3.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC v2] qemu: Support numad
On Wed, Mar 07, 2012 at 09:55:16PM +0800, Osier Yang wrote: numad is an user-level daemon that monitors NUMA topology and processes resource consumption to facilitate good NUMA resource alignment of applications/virtual machines to improve performance and minimize cost of remote memory latencies. It provides a pre-placement advisory interface, so significant processes can be pre-bound to nodes with sufficient available resources. More details: http://fedoraproject.org/wiki/Features/numad numad -w ncpus:memory_amount is the advisory interface numad provides currently. This patch add the support by introducing a bool XML element: numatune autonuma/ /numatune If it's specified, the number of vcpus and the current memory amount specified in domain XML will be used for numad command line (numad uses MB for memory amount): numad -w $num_of_vcpus:$current_memory_amount / 1024 The advisory nodeset returned from numad will be used to set domain process CPU affinity then. (e.g. qemuProcessInitCpuAffinity). If the user specifies both CPU affinity policy (e.g. (vcpu cpuset=1-10,^7,^84/vcpu) and XML indicating to use numad for the advisory nodeset, the specified CPU affinity will be ignored. I'm not sure that's a good idea. When we do dynamic generation of parts of libvirt XML, we tend to report in the XML what was generated, and if 2 parts contradict each other we shouldn't silently ignore it. eg, with VNC with autoport=yes we then report the generated port number. Similarly with cpu mode=host, we then report what the host CPU features were. So, if we want to auto-set placement for a guest we should likely do this via the vcpu element eg, Current mode where placement is completely static - Input XML: vcpu placement=static cpuset=1-10 / - Output XML: vcpu placement=static cpuset=1-10 / Or where we want to use numad: - Input XML: vcpu placement=auto/ - Output XML: vcpu placement=auto cpuset=1-10 / The current numad functionality you propose only sets the initial guest placement. Are we likely to have a mode in the future where numad will be called to update the placement periodically for existing guests ? If so, then placement would need to have more enum values. Regards, Daniel -- |: http://berrange.com -o-http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rpc: allow truncated return for virDomainGetCPUStats
On 03/07/2012 12:18 AM, Osier Yang wrote:
On 03/07/2012 02:46 PM, Osier Yang wrote:
On 03/07/2012 12:48 PM, Eric Blake wrote:
The RPC code assumed that the array returned by the driver would be
fully populated; that is, ncpus on entry resulted in ncpus * return
value on exit. However, while we don't support holes in the middle
of ncpus, we do want to permit the case of ncpus on entry being
longer than the array returned by the driver (that is, it should be
safe for the caller to pass ncpus=128 on entry, and the driver will
stop populating the array when it hits max_id).
/* The remote side did not send back any zero entries, so we have
- * to expand things back into a possibly sparse array.
+ * to expand things back into a possibly sparse array, where the
+ * tail of the array may be omitted.
*/
memset(params, 0, sizeof(*params) * nparams * ncpus);
+ ncpus = ret.params.params_len / ret.nparams;
for (cpu = 0; cpu ncpus; cpu++) {
int tmp = nparams;
remote_typed_param *stride =ret.params.params_val[cpu * ret.nparams];
Make sense, and ACK.
I realized that qemu_driver.c also needs this memset to guarantee that
unused slots are returned empty on success.
But do we want to add document to declare the returned array will
be truncated among the API implementation. Not neccessary though.
Perhaps something like:
* whole). Otherwise, @start_cpu represents which cpu to start
* with, and @ncpus represents how many consecutive processors to
* query, with statistics attributable per processor (such as
- * per-cpu usage).
+ * per-cpu usage). If @ncpus is larger than the number of host
+ * CPUs, the exceeded one(s) will be just ignored.
Good idea. Here's what I squashed in before pushing:
diff --git i/src/libvirt.c w/src/libvirt.c
index d98741b..39ebb52 100644
--- i/src/libvirt.c
+++ w/src/libvirt.c
@@ -18534,7 +18534,9 @@ error:
* whole). Otherwise, @start_cpu represents which cpu to start
* with, and @ncpus represents how many consecutive processors to
* query, with statistics attributable per processor (such as
- * per-cpu usage).
+ * per-cpu usage). If @ncpus is larger than the number of cpus
+ * available to query, then the trailing part of the array will
+ * be unpopulated.
*
* The remote driver imposes a limit of 128 @ncpus and 16 @nparams;
* the number of parameters per cpu should not exceed 16, but if you
@@ -18579,8 +18581,10 @@ error:
* number of populated @params, unless @ncpus was 1; and may be
* less than @nparams). The populated parameters start at each
* stride of @nparams, which means the results may be discontiguous;
- * any unpopulated parameters will be zeroed on success. The caller
- * is responsible for freeing any returned string parameters.
+ * any unpopulated parameters will be zeroed on success (this includes
+ * skipped elements if @nparams is too large, and tail elements if
+ * @ncpus is too large). The caller is responsible for freeing any
+ * returned string parameters.
*/
int virDomainGetCPUStats(virDomainPtr domain,
virTypedParameterPtr params,
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 538a419..ddb381a 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -12162,6 +12162,7 @@ qemuDomainGetPercpuStats(virDomainPtr domain,
if (virCgroupGetCpuacctPercpuUsage(group, buf))
goto cleanup;
pos = buf;
+memset(params, 0, nparams * ncpus);
if (max_id - start_cpu ncpus - 1)
max_id = start_cpu + ncpus - 1;
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Qemu, libvirt, and CPU models
On Tue, Mar 06, 2012 at 03:27:53PM -0300, Eduardo Habkost wrote: Hi, Sorry for the long message, but I didn't find a way to summarize the questions and issues and make it shorter. For people who don't know me: I have started to work recently on the Qemu CPU model code. I have been looking at how things work on libvirt+Qemu today w.r.t. CPU models, and I have some points I would like to understand better and see if they can be improved. I have two main points I would like to understand/discuss: 1) The relationship between libvirt's cpu_map.xml and the Qemu CPU model definitions. We have several areas of code in which we use CPU definitions - Reporting the host CPU definition (virsh capabilities) - Calculating host CPU compatibility / baseline definitions - Checking guest / host CPU compatibility - Configuring the guest CPU definition libvirt targets multiple platforms, and our CPU handling code is designed to be common sharable across all the libvirt drivers, VMWare, Xen, KVM, LXC, etc. Obviously for container based virt, only the host side of things is relevant. The libvirt CPU XML definition consists of - Model name - Vendor name - zero or more feature flags added/removed. A model name is basically just an alias for a bunch of feature flags, so that the CPU XML definitions are a) reasonably short b) have some sensible default baselines. The cpu_map.xml is the database of the CPU models that libvirt supports. We use this database to transform the CPU definition from the guest XML, into the hypervisor's own format. As luck would have it, the cpu_map.xml file contents match what QEMU has. This need not be the case though. If there is a model in the libvirt cpu_map.xml that QEMU doesn't know, we'll just pick the nearest matching QEMU cpu model specify the fature flags to compensate. We could go one step further and just write out a cpu.conf file that we load in QEMU with -loadconfig. On Xen we would use the cpu_map.xml to generate the CPUID masks that Xen expects. Similarly for VMWare. 2) How we could properly allow CPU models to be changed without breaking existing virtual machines? What is the scope of changes expected to CPU models ? 1) Qemu and cpu_map.xml I would like to understand how cpu_map.xml is supposed to be used, and how it is supposed to interact with the CPU model definitions provided by Qemu. More precisely: 1.1) Do we want to eliminate the duplication between the Qemu CPU definitions and cpu_map.xml? It isn't possible for us to the libvirt cpu_map.xml, since we need that across all our hypervisor targets. 1.1.1) If we want to eliminate the duplication, how can we accomplish that? What interfaces you miss, that Qemu could provide? 1.1.2) If the duplication has a purpose and you want to keep cpu_map.xml, then: - First, I would like to understand why libvirt needs cpu_map.xml? Is it part of the public interface of libvirt, or is it just an internal file where libvirt stores non-user-visible data? - How can we make sure there is no confusion between libvirt and Qemu about the CPU models? For example, what if cpu_map.xml says model 'Moo' has the flag 'foo' enabled, but Qemu disagrees? How do we guarantee that libvirt gets exactly what it expects from Qemu when it asks for a CPU model? We have -cpu ?dump today, but it's not the better interface we could have. Do you miss something in special in the Qemu-libvirt interface, to help on that? 1.2) About the probing of available features on the host system: Qemu has code specialized to query KVM about the available features, and to check what can be enabled and what can't be enabled in a VM. On many cases, the available features match exactly what is returned by the CPUID instruction on the host system, but there are some exceptions: - Some features can be enabled even when the host CPU doesn't support it (because they are completely emulated by KVM, e.g. x2apic). - On many other cases, the feature may be available but we have to check if Qemu+KVM are really able to expose it to the guest (many features work this way, as many depend on specific support by the KVM kernel module and/or Qemu). I suppose libvirt does want to check which flags can be enabled in a VM, as it already have checks for host CPU features (e.g. src/cpu/cpu_x86.c:x86Compute()). But I also suppose that libvirt doesn't want to duplicate the KVM feature probing code present on Qemu, and in this case we could have an interface where libvirt could query for the actually-available CPU features. Would it be useful for libvirt? What's the best way to expose this interface? 1.3) Some features are not plain CPU feature bits: e.g. level=X can be set in -cpu argument, and other features are enabled/disabled by exposing specific CPUID leafs and not just a feature bit (e.g. PMU CPUID leaf support). I
[libvirt] [PATCH] sanlock: Use STREQ_NULLABLE instead of STREQ on strings that may be null
The function sanlock_inquire can return NULL in the state string if the message consists only of a header. The return value is arbitrary and sent by the server. We should proceed carefully while touching such pointers. --- src/locking/lock_driver_sanlock.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c index 13940f1..f2623a0 100644 --- a/src/locking/lock_driver_sanlock.c +++ b/src/locking/lock_driver_sanlock.c @@ -828,7 +828,7 @@ static int virLockManagerSanlockRelease(virLockManagerPtr lock, return -1; } -if (STREQ(*state, )) +if (STREQ_NULLABLE(*state, )) VIR_FREE(*state); } @@ -871,7 +871,7 @@ static int virLockManagerSanlockInquire(virLockManagerPtr lock, return -1; } -if (STREQ(*state, )) +if (STREQ_NULLABLE(*state, )) VIR_FREE(*state); return 0; -- 1.7.3.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] sanlock: Fix condition left crippled while debugging
---
Extra context for review.
src/locking/lock_driver_sanlock.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/locking/lock_driver_sanlock.c
b/src/locking/lock_driver_sanlock.c
index f2623a0..d344d6a 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -669,73 +669,73 @@ static int virLockManagerSanlockAcquire(virLockManagerPtr
lock,
const char *state,
unsigned int flags,
int *fd)
{
virLockManagerSanlockPrivatePtr priv = lock-privateData;
struct sanlk_options *opt;
struct sanlk_resource **res_args;
int res_count;
bool res_free = false;
int sock = -1;
int rv;
int i;
virCheckFlags(VIR_LOCK_MANAGER_ACQUIRE_RESTRICT |
VIR_LOCK_MANAGER_ACQUIRE_REGISTER_ONLY, -1);
if (priv-res_count == 0
priv-hasRWDisks
driver-requireLeaseForDisks) {
virLockError(VIR_ERR_CONFIG_UNSUPPORTED, %s,
_(Read/write, exclusive access, disks were present, but
no leases specified));
return -1;
}
if (VIR_ALLOC(opt) 0) {
virReportOOMError();
return -1;
}
if (!virStrcpy(opt-owner_name, priv-vm_name, SANLK_NAME_LEN)) {
virLockError(VIR_ERR_INTERNAL_ERROR,
_(Domain name '%s' exceeded %d characters),
priv-vm_name, SANLK_NAME_LEN);
goto error;
}
-if (state STRNEQ(state, ) 0) {
+if (state STRNEQ(state, )) {
if ((rv = sanlock_state_to_args((char *)state,
res_count,
res_args)) 0) {
if (rv = -200)
virLockError(VIR_ERR_INTERNAL_ERROR,
_(Unable to parse lock state %s: error %d),
state, rv);
else
virReportSystemError(-rv,
_(Unable to parse lock state %s),
state);
goto error;
}
res_free = true;
} else {
res_args = priv-res_args;
res_count = priv-res_count;
}
VIR_DEBUG(Register sanlock %d, flags);
/* We only initialize 'sock' if we are in the real
* child process and we need it to be inherited
*
* If sock==-1, then sanlock auto-open/closes a
* temporary sock
*/
if (priv-vm_pid == getpid()
(sock = sanlock_register()) 0) {
if (sock = -200)
virLockError(VIR_ERR_INTERNAL_ERROR,
_(Failed to open socket to sanlock daemon: error
%d),
sock);
else
virReportSystemError(-sock, %s,
_(Failed to open socket to sanlock daemon));
goto error;
--
1.7.3.4
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC v2] qemu: Support numad
On 2012年03月07日 21:48, Daniel P. Berrange wrote: On Wed, Mar 07, 2012 at 09:55:16PM +0800, Osier Yang wrote: numad is an user-level daemon that monitors NUMA topology and processes resource consumption to facilitate good NUMA resource alignment of applications/virtual machines to improve performance and minimize cost of remote memory latencies. It provides a pre-placement advisory interface, so significant processes can be pre-bound to nodes with sufficient available resources. More details: http://fedoraproject.org/wiki/Features/numad numad -w ncpus:memory_amount is the advisory interface numad provides currently. This patch add the support by introducing a bool XML element: numatune autonuma/ /numatune If it's specified, the number of vcpus and the current memory amount specified in domain XML will be used for numad command line (numad uses MB for memory amount): numad -w $num_of_vcpus:$current_memory_amount / 1024 The advisory nodeset returned from numad will be used to set domain process CPU affinity then. (e.g. qemuProcessInitCpuAffinity). If the user specifies both CPU affinity policy (e.g. (vcpu cpuset=1-10,^7,^84/vcpu) and XML indicating to use numad for the advisory nodeset, the specified CPU affinity will be ignored. I'm not sure that's a good idea. When we do dynamic generation of parts of libvirt XML, we tend to report in the XML what was generated, and if 2 parts contradict each other we shouldn't silently ignore it. Agreed, v1 overrides the cpuset=1-10,^6, but I thought it could be confused for user to see things are different after domain is started. eg, with VNC with autoport=yes we then report the generated port number. Similarly withcpu mode=host, we then report what the host CPU features were. So, if we want to auto-set placement for a guest we should likely do this via thevcpu element eg, Current mode where placement is completely static - Input XML: vcpu placement=static cpuset=1-10 / - Output XML: vcpu placement=static cpuset=1-10 / Or where we want to use numad: - Input XML: vcpu placement=auto/ - Output XML: vcpu placement=auto cpuset=1-10 / I must admit this is much better. :-) The current numad functionality you propose only sets the initial guest placement. Are we likely to have a mode in the future where numad will be called to update the placement periodically for existing guests ? Very possiable, now numad just provides the advisory interface, in future I guess it could manage the placement dynamically. If so, then placement would need to have more enum values. I will post a v3 tomorrow. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC v2] qemu: Support numad
Note numad will attempt to manage / balance processes after they're launched, but the ideal case is libvirt pre-places them in a good spot and they never move On 03/07/2012 10:38 AM, Osier Yang wrote: On 2012年03月07日 21:48, Daniel P. Berrange wrote: On Wed, Mar 07, 2012 at 09:55:16PM +0800, Osier Yang wrote: numad is an user-level daemon that monitors NUMA topology and processes resource consumption to facilitate good NUMA resource alignment of applications/virtual machines to improve performance and minimize cost of remote memory latencies. It provides a pre-placement advisory interface, so significant processes can be pre-bound to nodes with sufficient available resources. More details: http://fedoraproject.org/wiki/Features/numad numad -w ncpus:memory_amount is the advisory interface numad provides currently. This patch add the support by introducing a bool XML element: numatune autonuma/ /numatune If it's specified, the number of vcpus and the current memory amount specified in domain XML will be used for numad command line (numad uses MB for memory amount): numad -w $num_of_vcpus:$current_memory_amount / 1024 The advisory nodeset returned from numad will be used to set domain process CPU affinity then. (e.g. qemuProcessInitCpuAffinity). If the user specifies both CPU affinity policy (e.g. (vcpu cpuset=1-10,^7,^84/vcpu) and XML indicating to use numad for the advisory nodeset, the specified CPU affinity will be ignored. I'm not sure that's a good idea. When we do dynamic generation of parts of libvirt XML, we tend to report in the XML what was generated, and if 2 parts contradict each other we shouldn't silently ignore it. Agreed, v1 overrides the cpuset=1-10,^6, but I thought it could be confused for user to see things are different after domain is started. eg, with VNC with autoport=yes we then report the generated port number. Similarly withcpu mode=host, we then report what the host CPU features were. So, if we want to auto-set placement for a guest we should likely do this via thevcpu element eg, Current mode where placement is completely static - Input XML: vcpu placement=static cpuset=1-10 / - Output XML: vcpu placement=static cpuset=1-10 / Or where we want to use numad: - Input XML: vcpu placement=auto/ - Output XML: vcpu placement=auto cpuset=1-10 / I must admit this is much better. :-) The current numad functionality you propose only sets the initial guest placement. Are we likely to have a mode in the future where numad will be called to update the placement periodically for existing guests ? Very possiable, now numad just provides the advisory interface, in future I guess it could manage the placement dynamically. If so, then placement would need to have more enum values. I will post a v3 tomorrow. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH RFC v2] qemu: Support numad
On 2012年03月07日 23:47, Bill Gray wrote: Note numad will attempt to manage / balance processes after they're launched, but the ideal case is libvirt pre-places them in a good spot and they never move So we can add something like dynamic as the new placement mode if the interface for query the current placement of a process available then. On 03/07/2012 10:38 AM, Osier Yang wrote: On 2012年03月07日 21:48, Daniel P. Berrange wrote: On Wed, Mar 07, 2012 at 09:55:16PM +0800, Osier Yang wrote: numad is an user-level daemon that monitors NUMA topology and processes resource consumption to facilitate good NUMA resource alignment of applications/virtual machines to improve performance and minimize cost of remote memory latencies. It provides a pre-placement advisory interface, so significant processes can be pre-bound to nodes with sufficient available resources. More details: http://fedoraproject.org/wiki/Features/numad numad -w ncpus:memory_amount is the advisory interface numad provides currently. This patch add the support by introducing a bool XML element: numatune autonuma/ /numatune If it's specified, the number of vcpus and the current memory amount specified in domain XML will be used for numad command line (numad uses MB for memory amount): numad -w $num_of_vcpus:$current_memory_amount / 1024 The advisory nodeset returned from numad will be used to set domain process CPU affinity then. (e.g. qemuProcessInitCpuAffinity). If the user specifies both CPU affinity policy (e.g. (vcpu cpuset=1-10,^7,^84/vcpu) and XML indicating to use numad for the advisory nodeset, the specified CPU affinity will be ignored. I'm not sure that's a good idea. When we do dynamic generation of parts of libvirt XML, we tend to report in the XML what was generated, and if 2 parts contradict each other we shouldn't silently ignore it. Agreed, v1 overrides the cpuset=1-10,^6, but I thought it could be confused for user to see things are different after domain is started. eg, with VNC with autoport=yes we then report the generated port number. Similarly withcpu mode=host, we then report what the host CPU features were. So, if we want to auto-set placement for a guest we should likely do this via thevcpu element eg, Current mode where placement is completely static - Input XML: vcpu placement=static cpuset=1-10 / - Output XML: vcpu placement=static cpuset=1-10 / Or where we want to use numad: - Input XML: vcpu placement=auto/ - Output XML: vcpu placement=auto cpuset=1-10 / I must admit this is much better. :-) The current numad functionality you propose only sets the initial guest placement. Are we likely to have a mode in the future where numad will be called to update the placement periodically for existing guests ? Very possiable, now numad just provides the advisory interface, in future I guess it could manage the placement dynamically. If so, then placement would need to have more enum values. I will post a v3 tomorrow. Regards, Osier -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] sanlock: Use STREQ_NULLABLE instead of STREQ on strings that may be null
On 03/07/2012 07:30 AM, Peter Krempa wrote: The function sanlock_inquire can return NULL in the state string if the message consists only of a header. The return value is arbitrary and sent by the server. We should proceed carefully while touching such pointers. --- src/locking/lock_driver_sanlock.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] sanlock: Fix condition left crippled while debugging
On 03/07/2012 07:36 AM, Peter Krempa wrote: --- Extra context for review. src/locking/lock_driver_sanlock.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Per-guest configurable user/group for QEMU processes
On Mon, 27 Feb 2012 12:48:55 -0300 Marcelo Cerri mhce...@linux.vnet.ibm.com wrote: Just one more point. I'd like to validate the direction that I'm getting. I updated the XML parse and replaced the seclabel member in virDomainDef with: size_t nseclabels; virSecurityLabelDefPtr *seclabels; I also added a model field in virSecurityLabelDef to identify the sec driver. So, my idea is to replace any access to the seclabel with a search by the model name. So, for example, instead of using secdef = def-seclabels; I'll use: secdef = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); virDomainDefGetSecurityLabelDef will search for a seclabel with the given model/name. I'm having to update too many parts in the code and I'd like to save some time if this is not the right direction. Regards, Marcelo Great. I think it is a good approach. The lack of an enclosing tag still bothers me. But, as you said, there's no serious problem not having it and I can live with that :) I believe the primary driver should be defined in qemu.conf, so I would like to replace the security_driver config with two new configs: primary_security_driver and additional_security_drivers. The last one would contain a list of security divers separated by commas, for example: primary_security_driver = apparmor additional_security_divers = dac,another_driver For device seclabel's, I intend to add a model attribute to specify which security driver is being overriding (if it's not given, the primary driver is considered). domain ... ... devices disk type='file' device='disk' source file='/path/to/image1' seclabel relabel='no' model='dac'/ /source ... /disk disk type='file' device='disk' source file='/path/to/image2' seclabel relabel='yes' model=selinux label system_u:object_r:shared_content_t:s0 /label /seclabel /source ... /disk ... /devices seclabel type='dynamic' model='selinux' baselabeltext/baselabel /seclabel seclabel type='static' model='dac' label501:501/label imagelabel501:501/imagelabel /seclabel /domain What do you think? Regards, Marcelo On 02/23/2012 07:34 PM, Daniel P. Berrange wrote: On Thu, Feb 23, 2012 at 06:38:45PM -0200, Marcelo Cerri wrote: On 02/23/2012 05:47 PM, Daniel P. Berrange wrote: On Thu, Feb 23, 2012 at 05:41:27PM -0200, Marcelo Cerri wrote: Hi, I'm starting working on an improvement for libvirt to be able to support per-guest configurable user and group IDs for QEMU processes. Currently, libvirt uses a configurable pair of user and group, which is defined in qemu.conf, for all qemu processes when running in privileged mode. This topic was already commented in qemu mailing list (http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00758.html) but, as this requires changes in libvirt API, I'd like to discuss what would be the best solution for it. A solution (as proposed in the link above) would be to extend the security driver model to allow multiple drivers. In this case, an example of the XML definition would be: ... seclabel type='dynamic' model='selinux' labelsystem_u:system_r:svirt_t:s0:c633,c712/label imagelabelsystem_u:object_r:svirt_image_t:s0:c633,c712/imagelabel /seclabel seclabel type='dynamic' model='dac' label102:102/label imagelabel102:102/imagelabel /seclabel ... I don't know if this is a clean solution because the usual option would be to enclose the block above in a seclabels tag. But as this would break the actual API, it's not viable. While it is true that we would ordinarily have an enclosing tag likeseclabels, there's no serious problem not having it. Just having two (or more)seclabel elements in a row is just fine, given our backwards compatibility requirements. So I think this option is just fine. I agree that this is a good solution, considering the XML compatibility. But, what about virDomainGetSecurityLabel? It could access the first security label or ignore the DAC driver (and maybe another function could be added to access the whole list of seclabels), but it doesn't seem to be the best solution. We can just keep virDomainGetSecurityLabel()/virNodeGetSecurityModel as only ever handling the primary security driver. Then add some new APIs which are more general int virNodeGetSecurityModelCount(virConnectPtr conn); int virNodeGetSecurityModelList(virConnectPtr conn, virSecurityModelPtr models, int nmodels); int virDomainGetSecurityLabelList(virDomainptr dom, virSecuriyLabelptr labels, int nlabels); Regards,
[libvirt] [PATCH] qemu: Don't parse device twice in attach/detach
Some nits are generated during XML parse (e.g. MAC address of
an interface); However, with current implementation, if we
are plugging a device both to persistent and live config,
we parse given XML twice: first time for live, second for config.
This is wrong then as the second time we are not guaranteed
to generate same values as we did for the first time.
To prevent that we need to create a copy of DeviceDefPtr;
This is done through format/parse process instead of writing
functions for deep copy as it is easier to maintain:
adding new field to any virDomain*DefPtr doesn't require change
of copying function.
---
diff to v1:
-Eric's review included
src/conf/domain_conf.c | 70 ++
src/conf/domain_conf.h |3 ++
src/libvirt_private.syms |1 +
src/qemu/qemu_driver.c | 38 ++---
4 files changed, 95 insertions(+), 17 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index b994718..42cfbd5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -14529,3 +14529,73 @@ virDomainNetFind(virDomainDefPtr def, const char
*device)
return net;
}
+
+virDomainDeviceDefPtr
+virDomainDeviceDefCopy(virCapsPtr caps,
+ const virDomainDefPtr def,
+ virDomainDeviceDefPtr src)
+{
+virDomainDeviceDefPtr ret = NULL;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+int flags = VIR_DOMAIN_XML_INACTIVE;
+char *xmlStr = NULL;
+int rc = -1;
+
+switch(src-type) {
+case VIR_DOMAIN_DEVICE_DISK:
+rc = virDomainDiskDefFormat(buf, src-data.disk, flags);
+break;
+case VIR_DOMAIN_DEVICE_LEASE:
+rc = virDomainLeaseDefFormat(buf, src-data.lease);
+break;
+case VIR_DOMAIN_DEVICE_FS:
+rc = virDomainFSDefFormat(buf, src-data.fs, flags);
+break;
+case VIR_DOMAIN_DEVICE_NET:
+rc = virDomainNetDefFormat(buf, src-data.net, flags);
+break;
+case VIR_DOMAIN_DEVICE_INPUT:
+rc = virDomainInputDefFormat(buf, src-data.input, flags);
+break;
+case VIR_DOMAIN_DEVICE_SOUND:
+rc = virDomainSoundDefFormat(buf, src-data.sound, flags);
+break;
+case VIR_DOMAIN_DEVICE_VIDEO:
+rc = virDomainVideoDefFormat(buf, src-data.video, flags);
+break;
+case VIR_DOMAIN_DEVICE_HOSTDEV:
+rc = virDomainHostdevDefFormat(buf, src-data.hostdev, flags);
+break;
+case VIR_DOMAIN_DEVICE_WATCHDOG:
+rc = virDomainWatchdogDefFormat(buf, src-data.watchdog, flags);
+break;
+case VIR_DOMAIN_DEVICE_CONTROLLER:
+rc = virDomainControllerDefFormat(buf, src-data.controller, flags);
+break;
+case VIR_DOMAIN_DEVICE_GRAPHICS:
+rc = virDomainGraphicsDefFormat(buf, src-data.graphics, flags);
+break;
+case VIR_DOMAIN_DEVICE_HUB:
+rc = virDomainHubDefFormat(buf, src-data.hub, flags);
+break;
+case VIR_DOMAIN_DEVICE_REDIRDEV:
+rc = virDomainRedirdevDefFormat(buf, src-data.redirdev, flags);
+break;
+default:
+virDomainReportError(VIR_ERR_INTERNAL_ERROR,
+ _(Copying definition of '%d' type
+ is not implemented yet.),
+ src-type);
+goto cleanup;
+}
+
+if (rc 0)
+goto cleanup;
+
+xmlStr = virBufferContentAndReset(buf);
+ret = virDomainDeviceDefParse(caps, def, xmlStr, flags);
+
+cleanup:
+VIR_FREE(xmlStr);
+return ret;
+}
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index 87b4103..1453822 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -1794,6 +1794,9 @@ void virDomainHostdevDefFree(virDomainHostdevDefPtr def);
void virDomainHubDefFree(virDomainHubDefPtr def);
void virDomainRedirdevDefFree(virDomainRedirdevDefPtr def);
void virDomainDeviceDefFree(virDomainDeviceDefPtr def);
+virDomainDeviceDefPtr virDomainDeviceDefCopy(virCapsPtr caps,
+ const virDomainDefPtr def,
+ virDomainDeviceDefPtr src);
int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info,
int type);
int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr);
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c44c617..a4cf199 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -287,6 +287,7 @@ virDomainDeviceAddressIsValid;
virDomainDeviceAddressPciMultiTypeFromString;
virDomainDeviceAddressPciMultiTypeToString;
virDomainDeviceAddressTypeToString;
+virDomainDeviceDefCopy;
virDomainDeviceDefFree;
virDomainDeviceDefParse;
virDomainDeviceInfoIterate;
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ddb381a..1d121cf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5563,7 +5563,7 @@
[libvirt] [PATCH] qemu: Fix startupPolicy for snapshot-revert
Currently, startupPolicy='requisite' was determining cold boot
by migrateForm != NULL. That means, if domain was started up
with migrateForm set we didn't require disk source path and allowed
it to be dropped. However, on snapshot-revert domain wasn't migrated
but according to documentation, requisite should drop disk source
as well.
---
Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=798938
src/qemu/qemu_driver.c| 16 +---
src/qemu/qemu_migration.c |2 +-
src/qemu/qemu_process.c |3 ++-
src/qemu/qemu_process.h |1 +
4 files changed, 13 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index ddb381a..b17b52c 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1358,7 +1358,7 @@ static virDomainPtr qemudDomainCreate(virConnectPtr conn,
const char *xml,
if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_MODIFY) 0)
goto cleanup; /* free the 'vm' we created ? */
-if (qemuProcessStart(conn, driver, vm, NULL,
+if (qemuProcessStart(conn, driver, vm, NULL, true,
(flags VIR_DOMAIN_START_PAUSED) != 0,
(flags VIR_DOMAIN_START_AUTODESTROY) != 0,
-1, NULL, NULL, VIR_NETDEV_VPORT_PROFILE_OP_CREATE)
0) {
@@ -4107,8 +4107,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn,
}
/* Set the migration source and start it up. */
-ret = qemuProcessStart(conn, driver, vm, stdio, true,
- false, *fd, path, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_RESTORE);
+ret = qemuProcessStart(conn, driver, vm, stdio, false, true,
+ false, *fd, path, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_RESTORE);
if (intermediatefd != -1) {
if (ret 0) {
@@ -4709,8 +4710,9 @@ qemuDomainObjStart(virConnectPtr conn,
}
}
-ret = qemuProcessStart(conn, driver, vm, NULL, start_paused,
- autodestroy, -1, NULL, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
+ret = qemuProcessStart(conn, driver, vm, NULL, true, start_paused,
+ autodestroy, -1, NULL, NULL,
+ VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
virDomainAuditStart(vm, booted, ret = 0);
if (ret = 0) {
virDomainEventPtr event =
@@ -10788,7 +10790,7 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
virDomainObjAssignDef(vm, config, false);
rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL,
- true, false, -1, NULL, snap,
+ false, true, false, -1, NULL, snap,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
virDomainAuditStart(vm, from-snapshot, rc = 0);
detail = VIR_DOMAIN_EVENT_STARTED_FROM_SNAPSHOT;
@@ -10878,7 +10880,7 @@ static int
qemuDomainRevertToSnapshot(virDomainSnapshotPtr snapshot,
if (event)
qemuDomainEventQueue(driver, event);
rc = qemuProcessStart(snapshot-domain-conn, driver, vm, NULL,
- paused, false, -1, NULL, NULL,
+ false, paused, false, -1, NULL, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_CREATE);
virDomainAuditStart(vm, from-snapshot, rc = 0);
if (rc 0) {
diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c
index 77d40c0..92d046a 100644
--- a/src/qemu/qemu_migration.c
+++ b/src/qemu/qemu_migration.c
@@ -1229,7 +1229,7 @@ qemuMigrationPrepareAny(struct qemud_driver *driver,
/* Start the QEMU daemon, with the same command-line arguments plus
* -incoming $migrateFrom
*/
-if (qemuProcessStart(dconn, driver, vm, migrateFrom, true,
+if (qemuProcessStart(dconn, driver, vm, migrateFrom, false, true,
true, dataFD[0], NULL, NULL,
VIR_NETDEV_VPORT_PROFILE_OP_MIGRATE_IN_START) 0) {
virDomainAuditStart(vm, migrated, false);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 7b99814..502de89 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -3072,6 +3072,7 @@ int qemuProcessStart(virConnectPtr conn,
struct qemud_driver *driver,
virDomainObjPtr vm,
const char *migrateFrom,
+ bool cold_boot,
bool start_paused,
bool autodestroy,
int stdin_fd,
@@ -3227,7 +3228,7 @@ int qemuProcessStart(virConnectPtr conn,
goto cleanup;
VIR_DEBUG(Checking for CDROM and floppy presence);
-if (qemuDomainCheckDiskPresence(driver, vm, migrateFrom != NULL) 0)
+if (qemuDomainCheckDiskPresence(driver, vm, cold_boot) 0)
goto cleanup;
[libvirt] [PATCH] util: fix build mingw (and all non-linux) build failure
ATTRIBUTE_UNUSED was accidentally forgotten on one arg of a stub
function for functionality that's not present on non-linux
platforms. This causes a non-linux build with
--enable-compile-warnings=error to fail.
---
Pushed under build-breaker rule.
src/util/pci.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index c8a5287..a6212f2 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -2219,7 +2219,8 @@ pciDeviceNetName(char *device_link_sysfs_path
ATTRIBUTE_UNUSED,
int
pciDeviceGetVirtualFunctionInfo(const char *vf_sysfs_device_path
ATTRIBUTE_UNUSED,
-char **pfname, int *vf_index ATTRIBUTE_UNUSED)
+char **pfname ATTRIBUTE_UNUSED,
+int *vf_index ATTRIBUTE_UNUSED)
{
pciReportError(VIR_ERR_INTERNAL_ERROR, _(pciDeviceGetVirtualFunctionInfo
is not supported on non-linux platforms));
--
1.7.7.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Fix startupPolicy for snapshot-revert
On 03/07/2012 11:19 AM, Michal Privoznik wrote: Currently, startupPolicy='requisite' was determining cold boot by migrateForm != NULL. That means, if domain was started up with migrateForm set we didn't require disk source path and allowed s/migrateForm/migrateFrom/ (twice) it to be dropped. However, on snapshot-revert domain wasn't migrated but according to documentation, requisite should drop disk source as well. --- Related BZ: https://bugzilla.redhat.com/show_bug.cgi?id=798938 src/qemu/qemu_driver.c| 16 +--- src/qemu/qemu_migration.c |2 +- src/qemu/qemu_process.c |3 ++- src/qemu/qemu_process.h |1 + 4 files changed, 13 insertions(+), 9 deletions(-) @@ -4107,8 +4107,9 @@ qemuDomainSaveImageStartVM(virConnectPtr conn, } /* Set the migration source and start it up. */ -ret = qemuProcessStart(conn, driver, vm, stdio, true, - false, *fd, path, NULL, VIR_NETDEV_VPORT_PROFILE_OP_RESTORE); +ret = qemuProcessStart(conn, driver, vm, stdio, false, true, + false, *fd, path, NULL, Yuck - we're starting to rack up so many bools that it's hard to tell which one is which. This patch can go in as-is, but I'd also like to see a followup patch that refactors things into using an 'unsigned int flags' with an internal enum for bit values (QEMU_START_COLD, QEMU_START_PAUSED, QEMU_START_AUTODESTROY, ...). ACK. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't parse device twice in attach/detach
On 03/07/2012 10:48 AM, Michal Privoznik wrote: Some nits are generated during XML parse (e.g. MAC address of s/nits/members/ an interface); However, with current implementation, if we are plugging a device both to persistent and live config, we parse given XML twice: first time for live, second for config. This is wrong then as the second time we are not guaranteed to generate same values as we did for the first time. To prevent that we need to create a copy of DeviceDefPtr; This is done through format/parse process instead of writing functions for deep copy as it is easier to maintain: adding new field to any virDomain*DefPtr doesn't require change of copying function. --- diff to v1: -Eric's review included except for in the commit message itself :) ACK. But Laine had comments on v1 as well, in case you want to wait for a second opinion on whether this needs any more work. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu: Don't parse device twice in attach/detach
On 03/07/2012 12:48 PM, Michal Privoznik wrote: Some nits are generated during XML parse (e.g. MAC address of an interface); However, with current implementation, if we are plugging a device both to persistent and live config, we parse given XML twice: first time for live, second for config. This is wrong then as the second time we are not guaranteed to generate same values as we did for the first time. To prevent that we need to create a copy of DeviceDefPtr; This is done through format/parse process instead of writing functions for deep copy as it is easier to maintain: adding new field to any virDomain*DefPtr doesn't require change of copying function. --- diff to v1: -Eric's review included src/conf/domain_conf.c | 70 ++ src/conf/domain_conf.h |3 ++ src/libvirt_private.syms |1 + src/qemu/qemu_driver.c | 38 ++--- 4 files changed, 95 insertions(+), 17 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index b994718..42cfbd5 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -14529,3 +14529,73 @@ virDomainNetFind(virDomainDefPtr def, const char *device) return net; } I still think there should be a comment added here saying something like: NB: virDomainDeviceDefCopy does a deep copy of only the parts of a DeviceDef that are valid when just the flag VIR_DOMAIN_XML_INACTIVE is set. This means that any part of the device xml that is conditionally parsed/formatted based on some other flag being set (or on the INACTIVE flag being reset) *will not* be copied to the destination. Caveat emptor. + +virDomainDeviceDefPtr +virDomainDeviceDefCopy(virCapsPtr caps, + const virDomainDefPtr def, + virDomainDeviceDefPtr src) Otherwise it looks like you've taken care of all of Eric's issues, and seems clean, so ACK from me (conditional on adding a comment documenting the limitations of the new copy function). -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/3 V7] add nodeGetCPUmap() for getting available CPU IDs in a cpumap.
On 03/07/2012 03:36 AM, Richard W.M. Jones wrote:
TBH I found the documentation for virDomainGetCPUStats to be very
confusing indeed. I couldn't really tell if virt-top is calling the
API correctly or not, so I simply used Fujitsu's code directly.
That's a shame about the documentation not being clear; anything we can
do to improve it?
There are basically 5 calling modes:
* Typical use sequence is below.
*
* getting total stats: set start_cpu as -1, ncpus 1
* virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0) = nparams
* params = calloc(nparams, sizeof (virTypedParameter))
* virDomainGetCPUStats(dom, params, nparams, -1, 1, 0) = total stats.
*
* getting per-cpu stats:
* virDomainGetCPUStats(dom, NULL, 0, 0, 0, 0) = ncpus
* virDomainGetCPUStats(dom, NULL, 0, 0, 1, 0) = nparams
* params = calloc(ncpus * nparams, sizeof (virTypedParameter))
* virDomainGetCPUStats(dom, params, nparams, 0, ncpus, 0) = per-cpu stats
*
3 of the calling modes (when params is NULL) are there to let you
determine how large to size your arrays; the remaining two modes exist
to query the total stats, and to query a slice of up to 128 per-cpu stats.
The number of total stats can be different from the number of per-cpu
stats (right now, it's 1 each for qemu, but I have a pending patch that
I will post later today that adds two new total stats with no per-cpu
counterpart).
When querying total stats, start_cpu is -1 and ncpus is 1 (this is a
hard-coded requirement), so the return value is the number of stats
populated.
When querying per-cpu stats, the single 'params' pointer is actually
representing a 2D array. So if you allocate params with 3x4 slots, and
call virDomainGetCPUStats(dom, params, 3, 0, 4, 0), but there are only 3
online CPUs and the result of the call is 2, then the result will be
laid out as:
params[0] = CPU0 stat 0
params[1] = CPU0 stat 1
params[2] = NULL
params[3] = CPU1 stat 0
params[4] = CPU1 stat 1
params[5] = NULL
params[6] = CPU2 stat 0
params[7] = CPU2 stat 1
params[8] = NULL
params[9] = NULL
params[10] = NULL
params[11] = NULL
Furthermore, if you have a beefy system with more than 128 cpus, you
have to break the call into chunks of 128 at a time, using start_cpu at
0, 128, and so forth.
Do you have any comments on whether this is correct or not?
http://git.annexia.org/?p=ocaml-libvirt.git;a=blob;f=libvirt/libvirt_c_oneoffs.c;h=f827707a77e6478129370fce67e46ae745b9be9a;hb=HEAD#l534
546
547 /* get percpu information */
548 NONBLOCKING (nparams = virDomainGetCPUStats(dom, NULL, 0, -1, 1, 0));
This gets the number of total params, but this might be different than
the number of per-cpu params. I think you want this to use
virDomainGetCPUStats(dom, NULL, 0, 0, 1, 0)
or even the maximum of the two, if you plan on combining total and
per-cpu usage into the same call.
549 CHECK_ERROR (nparams 0, conn, virDomainGetCPUStats);
550
551 if ((params = malloc(sizeof(*params) * nparams * 128)) == NULL)
Here, you are blindly sizing params based on the maximum supported by
the API. It might be more efficient to s/128/nr_pcpus/ or even MIN(128,
nr_pcpus). It would also be possible to use virDomainGetCPUStats(dom,
NULL, 0, 0, 0, 0), which should be the same or less than nr_pcpus, if
I'm correctly understanding how nr_pcpus was computed.
552 caml_failwith (virDomainGetCPUStats: malloc);
553
554 cpustats = caml_alloc (nr_pcpus, 0); /* cpustats: array of params(list
of typed_param) */
555 cpu = 0;
556 while (cpu nr_pcpus) {
557 ncpus = nr_pcpus - cpu 128 ? 128 : nr_pcpus - cpu;
Good, this looks like the correct way to divide things into slices of
128 cpus per read.
558
559 NONBLOCKING (r = virDomainGetCPUStats(dom, params, nparams, cpu,
ncpus, 0));
Here, nparams should be at least as large as the value from
virDomainGetCPUStats(dom, NULL, 0, 0, 1, 0), since there might be more
per-cpu stats than there are total stats. If nparams is too small, you
will be silently losing out on those extra stats.
560 CHECK_ERROR (r 0, conn, virDomainGetCPUStats);
561
562 for (i = 0; i ncpus; i++) {
563 /* list of typed_param: single linked list of param_nodes */
564 param_head = Val_emptylist; /* param_head: the head param_node of
list of typed_param */
565
566 if (params[i * nparams].type == 0) {
567 Store_field(cpustats, cpu + i, param_head);
568 continue;
569 }
570
571 for (j = nparams - 1; j = 0; j--) {
Here, I'd start the iteration on j = r - 1, since we already know r =
nparams, and that any slots beyond r are NULL.
572 pos = i * nparams + j;
573 if (params[pos].type == 0)
574 continue;
575
576 param_node = caml_alloc(2, 0); /* param_node: typed_param, next
param_node */
577 Store_field(param_node, 1, param_head);
578 param_head = param_node;
579
580 typed_param =
Re: [libvirt] [PATCHv2 11/15] virsh: add option aliases
On 03/06/2012 10:46 AM, Peter Krempa wrote: On 03/06/2012 01:34 AM, Eric Blake wrote: In the past, we have created some virsh options with less-than-stellar names. For back-compat reasons, those names must continue to parse, but we don't want to document them in help output. This introduces a new option type, an alias, which points to a canonical option name later in the option list. I'm actually quite impressed that our code has already been factored to do all option parsing through common entry points, such that I got this added in relatively few lines of code! * tools/virsh.c (VSH_OT_ALIAS): New option type. (opts_echo): Hook up an alias, for easy testing. (vshCmddefOptParse, vshCmddefHelp, vshCmddefGetOption): Allow for aliases. * tests/virshtest.c (mymain): Test new feature. --- Nice way to mask old mistakes and still support them. I'm wondering if this will not confuse people if their beloved arguments disappear suddenly from the docs. Maybe the help command could explicitly state aliases that exist for commands to avoid some confusion. I'll document the old name in the man page, but not in 'virsh help'. I'm leaning towards an ACK as it's better to encourage to use the fixed spelling. Does anyone object? Peter -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [Qemu-devel] Qemu, libvirt, and CPU models
Thanks a lot for the explanations, Daniel. Comments about specific items inline. On Wed, Mar 07, 2012 at 02:18:28PM +, Daniel P. Berrange wrote: I have two main points I would like to understand/discuss: 1) The relationship between libvirt's cpu_map.xml and the Qemu CPU model definitions. We have several areas of code in which we use CPU definitions - Reporting the host CPU definition (virsh capabilities) - Calculating host CPU compatibility / baseline definitions - Checking guest / host CPU compatibility - Configuring the guest CPU definition libvirt targets multiple platforms, and our CPU handling code is designed to be common sharable across all the libvirt drivers, VMWare, Xen, KVM, LXC, etc. Obviously for container based virt, only the host side of things is relevant. The libvirt CPU XML definition consists of - Model name - Vendor name - zero or more feature flags added/removed. A model name is basically just an alias for a bunch of feature flags, so that the CPU XML definitions are a) reasonably short b) have some sensible default baselines. The cpu_map.xml is the database of the CPU models that libvirt supports. We use this database to transform the CPU definition from the guest XML, into the hypervisor's own format. Understood. Makes sense. As luck would have it, the cpu_map.xml file contents match what QEMU has. This need not be the case though. If there is a model in the libvirt cpu_map.xml that QEMU doesn't know, we'll just pick the nearest matching QEMU cpu model specify the fature flags to compensate. Awesome. So, if Qemu and libvirt disagrees, libvirt will know that and add the necessary flags? That was my main worry. If disagreement between Qemu and libvirt is not a problem, it would make things much easier. ...but: Is that really implemented? I simply don't see libvirt doing that. I see code that calls -cpu ? to list the available CPU models, but no code calling -cpu ?dump, or parsing the Qemu CPU definition config file. I even removed some random flags from the Nehalem model on my machine (running Fedora 16), and no additional flags were added. We could go one step further and just write out a cpu.conf file that we load in QEMU with -loadconfig. Sounds good. Anyway, I want to make everything configurable on the cpudef config file configurable on the command-line too, so both options (command-line or config file) would work. On Xen we would use the cpu_map.xml to generate the CPUID masks that Xen expects. Similarly for VMWare. 2) How we could properly allow CPU models to be changed without breaking existing virtual machines? What is the scope of changes expected to CPU models ? We already have at least four cases, affecting different fields of the CPU definitions: A) Adding/removing flags. Exampes: - When the current set of flags is simply incorrect. See commit df07ec56 on qemu.git, where lots of flags that weren't supposed to be set were removed from some models. - When a new feature is now supported by Qemu+KVM and it's present on real-world CPUs, but our CPU definitions don't have the feature yet. e.g. x2apic, that is present on real-world Westmere CPUs but disabled on Qemu Westmere CPU definition. B) Changing level for some reason. One example: Conroe, Penrym and Nehalem have level=2, but need to have level=4 to make CPU topology work, so they have to be changed. C) Enabling/disabling or overriding specific CPUID leafs. This isn't even configurable on the config files today, but I plan to allow it to be configured, otherwise users won't be able to enable/disable some features that are probed by the guest by simply looking at a CPUID leaf (e.g. the 0xA CPUID leaf that contains PMU information). The PMU leaf is an example where a CPU looks different by simply using a different Qemu or kernel version, and libvirt can't control the visibility of that feature to the guest: - If you start a Virtual Machine using Qemu-1.0 today, with the pc-1.0 machine-type, the PMU CPUID leaf won't be visible to the guest (as Qemu-1.0 doesn't support the PMU leaf). - If you start a Virtual Machine using Qemu-1.1 in the future, using the pc-1.1 machine-type, with a recent kernel, the PMU CPUID leaf _will_ be visible to the guest (as the qemu.git master branch supports it). Up to now, it is OK because the machine-type in theory help us control the feature, but we have a problem on this case: - If you start a Virtual Machine using Qemu-1.1 in the future, using the pc-1.1 machine-type, using exactly the same command-line as above, but using an old kernel, the PMU CPUID leaf will _not_ be visible to the guest. 1) Qemu and cpu_map.xml I would like to understand how cpu_map.xml is supposed to be used, and how it is supposed to interact with the CPU model definitions provided by Qemu. More precisely: 1.1) Do we want to eliminate the
Re: [libvirt] [Qemu-devel] Qemu, libvirt, and CPU models
On 03/07/2012 03:26 PM, Eduardo Habkost wrote: Thanks a lot for the explanations, Daniel. Comments about specific items inline. - How can we make sure there is no confusion between libvirt and Qemu about the CPU models? For example, what if cpu_map.xml says model 'Moo' has the flag 'foo' enabled, but Qemu disagrees? How do we guarantee that libvirt gets exactly what it expects from Qemu when it asks for a CPU model? We have -cpu ?dump today, but it's not the better interface we could have. Do you miss something in special in the Qemu-libvirt interface, to help on that? So, it looks like either I am missing something on my tests or libvirt is _not_ probing the Qemu CPU model definitions to make sure libvirt gets all the features it expects. Also, I would like to ask if you have suggestions to implement the equivalent of -cpu ?dump in a more friendly and extensible way. Would a QMP command be a good alternative? Would a command-line option with json output be good enough? I'm not sure where we are are using -cpu ?dump, but it sounds like we should be. (Do we have any case of capability-querying being made using QMP before starting any actual VM, today?) Right now, we have two levels of queries - the 'qemu -help' and 'qemu -device ?' output is gathered up front (we really need to patch things to cache that, rather than repeating it for every VM start). Then we start qemu with -S, query QMP, all before starting the guest (qemu -S is in fact necessary for setting some options that cannot be set in the current CLI but can be set via the monitor) - but right now that is the only point where we query QMP capabilities. If QMP can alter the CPU model prior to the initial start of the guest, then that would be a sufficient interface. But I'm worried that once we start qemu, even with qemu -S, that it's too late to alter the CPU model in use by that guest, and that libvirt should instead start querying these things in advance. We definitely want a machine-parseable construct, so querying over QMP rather than '-cpu ?dump' sounds like it might be nicer, but it would also be more work to set up libvirt to do a dry-run query of QMP capabilities without also starting a real guest. But what about the features that are not available on the host CPU, libvirt will think it can't be enabled, but that _can_ be enabled? x2apic seems to be the only case today, but we may have others in the future. That's where having an interface to probe qemu to see what capabilities are possible for any given cpu model would be worthwhile, so that libvirt can correlate the feature sets properly. That answers most of my questions about how libvirt would handle changes on CPU models. Now we need good mechanisms that allow libvirt to do that. If you have specific requirements or suggestions in mind, please let me know. I'll let others chime in with more responses, but I do appreciate you taking the time to coordinate this. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 09/15] xml: use better types for memory values
On 03/06/2012 09:34 AM, Peter Krempa wrote:
On 03/06/2012 01:34 AM, Eric Blake wrote:
Using 'unsigned long' for memory values is risky on 32-bit platforms,
as a PAE guest can have more than 4GiB memory. Our API is
(unfortunately) locked at 'unsigned long' and a scale of 1024, but
the rest of our system should consistently use 64-bit values,
especially since the previous patch centralized overflow checking.
+++ b/src/xenxs/xen_sxpr.c
@@ -1101,9 +1133,15 @@ cleanup:
static
-int xenXMConfigSetInt(virConfPtr conf, const char *setting, long l) {
+int xenXMConfigSetInt(virConfPtr conf, const char *setting, long long
l) {
virConfValuePtr value = NULL;
+if ((long) l != l) {
+XENXS_ERROR(VIR_ERR_INTERNAL_ERROR,
I suppose the VIR_ERR_INTERNAL_ERROR is intentional.
Yes - the libvirt.c interface takes 'unsigned long', and widening that
to long long should be reversible so we should never hit this error.
+_(integer overflow in trying to store %lld to %s),
+l, setting);
+return -1;
+}
if (VIR_ALLOC(value) 0) {
virReportOOMError();
return -1;
ACK,
Peter
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 14/15] virsh: improve storage unit parsing
On 03/07/2012 05:45 AM, Peter Krempa wrote:
On 03/06/2012 01:34 AM, Eric Blake wrote:
Now can now do:
virsh vol-resize $vol 10M
virsh blockresize $dom $vol 10M
to get both interfaces to resize to 10MiB. The remaining wart
is that vol-resize defaults to bytes, but blockresize defaults
to KiB, but we can't break existing scripts; oh well, it's no
worse than the same wart of the underlying virDomainBlockResize.
-capacity = -capacity;
-} else {
-if (cmdVolSize(capacityStr,capacity) 0) {
-vshError(ctl, _(Malformed size %s), capacityStr);
-goto cleanup;
-}
+capacityStr++;
This shift in the string discards the minus sign in error/success
messages, but their meaning remains correct.
The old code was wrong for negating things - the public API is
documented as always requiring a non-negative value (the combination of
delta and shrink implies a reduction, no negative needed). So this is
actually a bug fix.
This was probably added as a safety measure and I'd prefer if we would
require the --shrink flag to mark that the user is sure of what he's
doing, although it should be obvious enough from the minus sign. (I
honestly would prefer a docs change but I'm afraid someone could get mad
at us if he accidentaly corrupts his images.)
My latest version now requires both --delta and --shrink to be present
before ignoring a negative sign.
ACK, if you add the check for --shrink.
Peter
--
Eric Blake ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2 15/15] virsh: improve memory unit parsing
On 03/07/2012 06:14 AM, Peter Krempa wrote: On 03/06/2012 01:34 AM, Eric Blake wrote: The last vestige of the inaccurate 'kilobytes' when we meant 1024 is now gone. And virsh is now useful for setting memory in units other than KiB. +static int +cmdMemtuneGetSize(const vshCmd *cmd, const char *name, long long *value) As this is a helper function rename it please to vshMemtuneGetSize to avoid confusion with command functions. Sure. ACK with the name change. Thanks for the review. Nits fixed, and series pushed. -- Eric Blake ebl...@redhat.com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] virsh: Use option alias for outmoded --persistent
On 03/07/2012 03:17 AM, Osier Yang wrote:
Since VIR_DOMAIN_AFFECT_{LIVE,CONFIG,CURRENT} was created,
all new virsh commands use --config to represents the
persistent changing. This patch add --config option
for the old commands which still use --persistent,
and --persistent is now alias of --config.
tools/virsh.c: (use --config, and --persistent is
alias of --config now).
cmdDomIfSetLink, cmdDomIfGetLink, cmdAttachDevice,
cmdDetachDevice, cmdUpdateDevice, cmdAttachInterface,
cmdDetachInterface, cmdAttachDisk, cmdDetachDisk
toos/virsh.pod: Update docs of the changed commands, and
add some missed docs for --config (detach-interface,
detach-disk, and detach-device).
---
tools/virsh.c | 51 ++-
tools/virsh.pod | 36 ++--
2 files changed, 52 insertions(+), 35 deletions(-)
The code looks fine.
Per the previous patches, we should document in virsh.pod that older
versions of virsh understood --persistent, so it's probably worth a v2
to make sure we like the docs.
--
Eric Blake ebl...@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
