[libvirt] simple LXC/libvirt busybox container (Unable to get cgroup)

[Tony Risinger]

i'm trying to get even the simplest busybox container with libvirt+LXC
with very limited success.  I feel l am missing something supremely
simple for me to be hung on this for weeks.

i dont see anything interesting in domain log, but getting this error
from LIBVIRT_DEBUG=1 libvirtd:

05:27:56.113: error : lxcDomainGetInfo:462 : internal error Unable to
get cgroup for arch-nano
05:27:56.113: debug : virDomainFree:2004 : domain=0x81d8e68
05:27:56.113: debug : virUnrefDomain:422 : unref domain 0x81d8e68 arch-nano 1
05:27:56.113: debug : virReleaseDomain:376 : release domain 0x81d8e68 arch-nano
05:27:56.113: debug : virReleaseDomain:392 : unref connection 0x81dc0f0 2
05:27:56.113: debug : remoteSerializeError:141 : prog=536903814 ver=1
proc=16 type=1 serial=4, msg=internal error Unable to get cgroup for
arch-nano

i've been using this root filesystem layout:

[r...@phs-001 arch-nano]# tree
.
|-- bin
|   |-- cat - ../sbin/busybox
|   |-- chdir - ../sbin/busybox
|   |-- chmod - ../sbin/busybox
|   |-- ls - ../sbin/busybox
|   |-- rm - ../sbin/busybox
|   |-- sh - ../sbin/busybox
|   `-- vi - ../sbin/busybox
|-- dev
|   `-- pts
|-- etc
|-- proc
|-- sbin
|   |-- busybox
|   `-- init - busybox
`-- sys

all folders besides /bin and /sbin were created by libvirt.  i tried
using the /sbin/init script previously suggested:

#!/sbin/busybox
sh

but i get a similar results either way (script/symlink):

 8173 ?Ss 0:00 /usr/lib/libvirt-git/libvirt_lxc --name
arch-nano --console 11 --background
 8175 pts/0Ss+0:00 init
 8177 ?Ss 0:00 init
 8181 ?Zs 0:00 [init] defunct
 8182 ?Zs 0:00 [init] defunct
 8183 ?Zs 0:00 [init] defunct


busybox init doc says that without an /etc/inittab, sh will be
started on /dev/tty2... im am using this config:

domain type='lxc'
  namearch-nano/name
  memory50/memory
  os
typeexe/type
init/sbin/init/init
  /os
  devices
filesystem type='mount'
  source dir='/vps/dom/arch-nano'/
  target dir='/'/
/filesystem
  /devices
/domain

on this config i tried removing the console/serial section altogether,
but i attempted many configurations of serial/console, including
changing the target to 2, in attempt to match busybox default.

[r...@phs-001 arch-nano]# virsh -c lxc:/// console arch-nano
error: Unable to get domain status
error: internal error Unable to get cgroup for arch-nano

[r...@phs-001 arch-nano]# mount | grep cgroup
none on /cgroup type cgroup (rw)

if anyone can please point out what i am doing wrong to not be able to
move the root and get a console, i'd greatly appreciate it.  ive been
really stuck on this; i'd rather not write a bunch of scripts/wrappers
for lxc-* tools when libvirt does it all splendidly already!

libvirt 0.7.4+
kernel 2.6.32 (am i missing a CONFIG_*?)

thanks for your time

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] qemu smbios options in domain xml format

[Paolo Bonzini]

On 12/09/2009 10:47 PM, Phillip Balli wrote:

Hello,

I could not find any explicit mention of support for the -smbios
options which are present in qemu 0.10.5+ in the domain xml format or
the qemu/kvm hypervisor driver pages. Is there some way to specify
commands and options which are not described in the xml config by
adjusting something in the way libvirt processes the xml? Or is
providing smbios options just not supported currently?


No, it's currently not supported.

Paolo

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] simple LXC/libvirt busybox container (Unable to get cgroup)

[Ryota Ozaki]

On Thu, Dec 10, 2009 at 5:22 PM, Tony Risinger
sweetsinsemi...@gmail.com wrote:
 i'm trying to get even the simplest busybox container with libvirt+LXC
 with very limited success.  I feel l am missing something supremely
 simple for me to be hung on this for weeks.

 i dont see anything interesting in domain log, but getting this error
 from LIBVIRT_DEBUG=1 libvirtd:

 05:27:56.113: error : lxcDomainGetInfo:462 : internal error Unable to
 get cgroup for arch-nano
 05:27:56.113: debug : virDomainFree:2004 : domain=0x81d8e68
 05:27:56.113: debug : virUnrefDomain:422 : unref domain 0x81d8e68 arch-nano 1
 05:27:56.113: debug : virReleaseDomain:376 : release domain 0x81d8e68 
 arch-nano
 05:27:56.113: debug : virReleaseDomain:392 : unref connection 0x81dc0f0 2
 05:27:56.113: debug : remoteSerializeError:141 : prog=536903814 ver=1
 proc=16 type=1 serial=4, msg=internal error Unable to get cgroup for
 arch-nano

 i've been using this root filesystem layout:

 [r...@phs-001 arch-nano]# tree
 .
 |-- bin
 |   |-- cat - ../sbin/busybox
 |   |-- chdir - ../sbin/busybox
 |   |-- chmod - ../sbin/busybox
 |   |-- ls - ../sbin/busybox
 |   |-- rm - ../sbin/busybox
 |   |-- sh - ../sbin/busybox
 |   `-- vi - ../sbin/busybox
 |-- dev
 |   `-- pts
 |-- etc
 |-- proc
 |-- sbin
 |   |-- busybox
 |   `-- init - busybox
 `-- sys

 all folders besides /bin and /sbin were created by libvirt.  i tried
 using the /sbin/init script previously suggested:

 #!/sbin/busybox
 sh

 but i get a similar results either way (script/symlink):

  8173 ?        Ss     0:00 /usr/lib/libvirt-git/libvirt_lxc --name
 arch-nano --console 11 --background
  8175 pts/0    Ss+    0:00 init
  8177 ?        Ss     0:00 init
  8181 ?        Zs     0:00 [init] defunct
  8182 ?        Zs     0:00 [init] defunct
  8183 ?        Zs     0:00 [init] defunct


 busybox init doc says that without an /etc/inittab, sh will be
 started on /dev/tty2... im am using this config:

 domain type='lxc'
  namearch-nano/name
  memory50/memory
  os
    typeexe/type
    init/sbin/init/init
  /os
  devices
    filesystem type='mount'
      source dir='/vps/dom/arch-nano'/
      target dir='/'/
    /filesystem
  /devices
 /domain

 on this config i tried removing the console/serial section altogether,
 but i attempted many configurations of serial/console, including
 changing the target to 2, in attempt to match busybox default.

 [r...@phs-001 arch-nano]# virsh -c lxc:/// console arch-nano
 error: Unable to get domain status
 error: internal error Unable to get cgroup for arch-nano

 [r...@phs-001 arch-nano]# mount | grep cgroup
 none on /cgroup type cgroup (rw)

 if anyone can please point out what i am doing wrong to not be able to
 move the root and get a console, i'd greatly appreciate it.  ive been
 really stuck on this; i'd rather not write a bunch of scripts/wrappers
 for lxc-* tools when libvirt does it all splendidly already!

I'm successfully using lxc via lbivirt. I doubt ns subsystem of cgroups
because I usually disable it that is the difference between my
configuration and yours and if I enable it I also get the same error.

So could you try without ns? like:
  mount -t cgroup -o memory,devices,cpu,cpuacct none /cgroup

I think libvirt lxc still contains a problem around ns subsystem.

  ozaki-r


 libvirt 0.7.4+
 kernel 2.6.32 (am i missing a CONFIG_*?)

 thanks for your time

 --
 Libvir-list mailing list
 Libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/5] reload iptables rules simply by re-adding them

[Mark McLoughlin]

Currently, when we add iptables rules, we keep them on a list so that
we can easily reload them on e.g. 'service libvirtd reload'.

However, we don't save this list to disk, so if libvirtd is restarted
we lose the ability to reload the rules.

The fix is simple - just re-add the damn things on reload.

Note, we delete the rules before re-adding them, just like the current
behaviour of iptRulesReload().

* src/network/bridge_driver.c: re-add the iptables rules on reload.
---
 src/network/bridge_driver.c |   30 --
 1 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0342aa0..766f8cd 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -96,6 +96,8 @@ static int networkShutdownNetworkDaemon(virConnectPtr conn,
   struct network_driver *driver,
   virNetworkObjPtr network);
 
+static void networkReloadIptablesRules(struct network_driver *driver);
+
 static struct network_driver *driverState = NULL;
 
 
@@ -291,12 +293,7 @@ networkReload(void) {
  driverState-networks,
  driverState-networkConfigDir,
  driverState-networkAutostartDir);
-
- if (driverState-iptables) {
-VIR_INFO0(_(Reloading iptables rules\n));
-iptablesReloadRules(driverState-iptables);
-}
-
+networkReloadIptablesRules(driverState);
 networkAutostartConfigs(driverState);
 networkDriverUnlock(driverState);
 return 0;
@@ -812,6 +809,27 @@ networkRemoveIptablesRules(struct network_driver *driver,
 iptablesSaveRules(driver-iptables);
 }
 
+static void
+networkReloadIptablesRules(struct network_driver *driver)
+{
+unsigned int i;
+
+VIR_INFO0(_(Reloading iptables rules));
+
+for (i = 0 ; i  driver-networks.count ; i++) {
+virNetworkObjLock(driver-networks.objs[i]);
+
+if (virNetworkObjIsActive(driver-networks.objs[i])) {
+networkRemoveIptablesRules(driver, driver-networks.objs[i]);
+if (!networkAddIptablesRules(NULL, driver, 
driver-networks.objs[i])) {
+/* failed to add but already logged */
+}
+}
+
+virNetworkObjUnlock(driver-networks.objs[i]);
+}
+}
+
 /* Enable IP Forwarding. Return 0 for success, -1 for failure. */
 static int
 networkEnableIpForwarding(void)
-- 
1.6.5.2

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 2/5] reload iptables rules on libvirtd restart

[Mark McLoughlin]

This is the expected behaviour, I think - reloading libvirtd should
be a subset of restarting it.

Note, we reload the rules after we've determined which networks
are active (because we only add the rules for active networks)
and before we start autostart networks (to avoid re-adding the
rules).

* src/network/bridge_driver.c: reload iptables rules on startup
---
 src/network/bridge_driver.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 766f8cd..d5cab71 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -259,6 +259,7 @@ networkStartup(int privileged) {
 goto error;
 
 networkFindActiveConfigs(driverState);
+networkReloadIptablesRules(driverState);
 networkAutostartConfigs(driverState);
 
 networkDriverUnlock(driverState);
-- 
1.6.5.2

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/5] remove all traces of lokkit support

[Mark McLoughlin]

Long ago we tried to use Fedora's lokkit utility in order to register
our iptables rules so that 'service iptables restart' would
automatically load our rules.

There was one fatal flaw - if the user had configured iptables without
lokkit, then we would clobber that configuration by running lokkit.

We quickly disabled lokkit support, but never removed it. Let's do
that now.

The 'my virtual network stops working when I restart iptables' still
remains. For all the background on this saga, see:

  https://bugzilla.redhat.com/227011

* src/util/iptables.c: remove lokkit support

* configure.in: remove --enable-lokkit

* libvirt.spec.in: remove the dirs used only for saving rules for lokkit

* src/Makefile.am: ditto

* src/libvirt_private.syms, src/network/bridge_driver.c,
  src/util/iptables.h: remove references to iptablesSaveRules
---
 configure.in|   21 
 libvirt.spec.in |3 -
 src/Makefile.am |4 -
 src/libvirt_private.syms|1 -
 src/network/bridge_driver.c |3 -
 src/util/iptables.c |  218 ---
 src/util/iptables.h |1 -
 7 files changed, 0 insertions(+), 251 deletions(-)

diff --git a/configure.in b/configure.in
index 8d21207..fe9834d 100644
--- a/configure.in
+++ b/configure.in
@@ -269,27 +269,6 @@ if test x$with_rhel5_api = xyes; then
AC_DEFINE([WITH_RHEL5_API], [1], [whether building for the RHEL-5 API])
 fi
 
-dnl
-dnl ensure that Fedora's system-config-firewall knows
-dnl about libvirt's iptables rules
-dnl
-AC_ARG_ENABLE([iptables-lokkit],
-  [AC_HELP_STRING([--enable-iptables-lokkit=no/yes/check],
-  [enable registering libvirt's iptables rules with Fedora's lokkit])],
- [],[enable_iptables_lokkit=check])
-if test x$enable_iptables_lokkit != xno; then
-   AC_PATH_PROG([LOKKIT_PATH],[lokkit], [], [/usr/sbin:$PATH])
-fi
-
-if test x$enable_iptables_lokkit = xyes -a x$LOKKIT_PATH = x; then
-   AC_MSG_ERROR([Cannot find lokkit and --enable-iptables-lokkit specified])
-fi
-
-if test x$LOKKIT_PATH != x; then
-   AC_DEFINE([ENABLE_IPTABLES_LOKKIT], [], [whether support for Fedora's 
lokkit is enabled])
-   AC_DEFINE_UNQUOTED([LOKKIT_PATH], $LOKKIT_PATH, [path to lokkit binary])
-fi
-
 AC_PATH_PROG([IPTABLES_PATH], [iptables], /sbin/iptables, [/usr/sbin:$PATH])
 AC_DEFINE_UNQUOTED([IPTABLES_PATH], $IPTABLES_PATH, [path to iptables 
binary])
 
diff --git a/libvirt.spec.in b/libvirt.spec.in
index 408ad05..dd067ad 100644
--- a/libvirt.spec.in
+++ b/libvirt.spec.in
@@ -710,9 +710,6 @@ fi
 %if %{with_network}
 %dir %{_localstatedir}/run/libvirt/network/
 %dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/network/
-%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/iptables/
-%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/iptables/filter/
-%dir %attr(0700, root, root) %{_localstatedir}/lib/libvirt/iptables/nat/
 %endif
 
 %if %{with_qemu}
diff --git a/src/Makefile.am b/src/Makefile.am
index e5d8933..b639915 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -883,8 +883,6 @@ if WITH_UML
$(MKDIR_P) $(DESTDIR)$(localstatedir)/run/libvirt/uml
 endif
 if WITH_NETWORK
-   $(MKDIR_P) $(DESTDIR)$(localstatedir)/lib/libvirt/iptables/filter
-   $(MKDIR_P) $(DESTDIR)$(localstatedir)/lib/libvirt/iptables/nat
$(MKDIR_P) $(DESTDIR)$(localstatedir)/lib/libvirt/network
$(MKDIR_P) $(DESTDIR)$(localstatedir)/run/libvirt/network
$(MKDIR_P) $(DESTDIR)$(sysconfdir)/libvirt/qemu/networks/autostart
@@ -921,8 +919,6 @@ if WITH_NETWORK
rm -f $(DESTDIR)$(sysconfdir)/libvirt/qemu/networks/default.xml
rmdir $(DESTDIR)$(sysconfdir)/libvirt/qemu/networks/autostart || :
rmdir $(DESTDIR)$(sysconfdir)/libvirt/qemu/networks || :
-   rmdir $(DESTDIR)$(localstatedir)/lib/libvirt/iptables/filter ||:
-   rmdir $(DESTDIR)$(localstatedir)/lib/libvirt/iptables/nat ||:
rmdir $(DESTDIR)$(localstatedir)/lib/libvirt/network ||:
rmdir $(DESTDIR)$(localstatedir)/run/libvirt/network ||:
 endif
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 58f99fb..8d64b15 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -247,7 +247,6 @@ iptablesRemoveForwardRejectIn;
 iptablesRemoveForwardRejectOut;
 iptablesRemoveTcpInput;
 iptablesRemoveUdpInput;
-iptablesSaveRules;
 
 
 # libvirt_internal.h
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index d5cab71..abee78c 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -752,8 +752,6 @@ networkAddIptablesRules(virConnectPtr conn,
  !networkAddRoutingIptablesRules(conn, driver, network))
 goto err8;
 
-iptablesSaveRules(driver-iptables);
-
 return 1;
 
  err8:
@@ -807,7 +805,6 @@ networkRemoveIptablesRules(struct network_driver *driver,
 iptablesRemoveTcpInput(driver-iptables, network-def-bridge, 53);
 

[libvirt] [PATCH 4/5] remove iptablesReloadRules() and related code

[Mark McLoughlin]

We don't use this method of reloading rules anymore, so we can just
kill the code.

This simplifies things a lot because we no longer need to keep a
table of the rules we've added.

* src/util/iptables.c: kill iptablesReloadRules()
---
 src/libvirt_private.syms |1 -
 src/util/iptables.c  |  155 +-
 src/util/iptables.h  |2 -
 3 files changed, 3 insertions(+), 155 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8d64b15..e5ba365 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -237,7 +237,6 @@ iptablesAddTcpInput;
 iptablesAddUdpInput;
 iptablesContextFree;
 iptablesContextNew;
-iptablesReloadRules;
 iptablesRemoveForwardAllowCross;
 iptablesRemoveForwardAllowIn;
 iptablesRemoveForwardAllowOut;
diff --git a/src/util/iptables.c b/src/util/iptables.c
index 8ac7786..3c02ea6 100644
--- a/src/util/iptables.c
+++ b/src/util/iptables.c
@@ -54,18 +54,8 @@ enum {
 
 typedef struct
 {
-char  *rule;
-const char **argv;
-intcommand_idx;
-} iptRule;
-
-typedef struct
-{
 char  *table;
 char  *chain;
-
-int  nrules;
-iptRule *rules;
 } iptRules;
 
 struct _iptablesContext
@@ -76,82 +66,10 @@ struct _iptablesContext
 };
 
 static void
-iptRuleFree(iptRule *rule)
-{
-VIR_FREE(rule-rule);
-
-if (rule-argv) {
-int i = 0;
-while (rule-argv[i])
-VIR_FREE(rule-argv[i++]);
-VIR_FREE(rule-argv);
-}
-}
-
-static int
-iptRulesAppend(iptRules *rules,
-   char *rule,
-   const char **argv,
-   int command_idx)
-{
-if (VIR_REALLOC_N(rules-rules, rules-nrules+1)  0) {
-int i = 0;
-while (argv[i])
-VIR_FREE(argv[i++]);
-VIR_FREE(argv);
-return ENOMEM;
-}
-
-rules-rules[rules-nrules].rule= rule;
-rules-rules[rules-nrules].argv= argv;
-rules-rules[rules-nrules].command_idx = command_idx;
-
-rules-nrules++;
-
-return 0;
-}
-
-static int
-iptRulesRemove(iptRules *rules,
-   char *rule)
-{
-int i;
-
-for (i = 0; i  rules-nrules; i++)
-if (STREQ(rules-rules[i].rule, rule))
-break;
-
-if (i = rules-nrules)
-return EINVAL;
-
-iptRuleFree(rules-rules[i]);
-
-memmove(rules-rules[i],
-rules-rules[i+1],
-(rules-nrules - i - 1) * sizeof (iptRule));
-
-rules-nrules--;
-
-return 0;
-}
-
-static void
 iptRulesFree(iptRules *rules)
 {
-int i;
-
 VIR_FREE(rules-table);
 VIR_FREE(rules-chain);
-
-if (rules-rules) {
-for (i = 0; i  rules-nrules; i++)
-iptRuleFree(rules-rules[i]);
-
-VIR_FREE(rules-rules);
-
-rules-nrules = 0;
-}
-
 VIR_FREE(rules);
 }
 
@@ -170,9 +88,6 @@ iptRulesNew(const char *table,
 if (!(rules-chain = strdup(chain)))
 goto error;
 
-rules-rules = NULL;
-rules-nrules = 0;
-
 return rules;
 
  error:
@@ -186,9 +101,8 @@ iptablesAddRemoveRule(iptRules *rules, int action, const 
char *arg, ...)
 va_list args;
 int retval = ENOMEM;
 const char **argv;
-char *rule = NULL;
 const char *s;
-int n, command_idx;
+int n;
 
 n = 1 + /* /sbin/iptables  */
 2 + /*   --table foo   */
@@ -215,9 +129,7 @@ iptablesAddRemoveRule(iptRules *rules, int action, const 
char *arg, ...)
 if (!(argv[n++] = strdup(rules-table)))
 goto error;
 
-command_idx = n;
-
-if (!(argv[n++] = strdup(--insert)))
+if (!(argv[n++] = strdup(action == ADD ? --insert : --delete)))
 goto error;
 
 if (!(argv[n++] = strdup(rules-chain)))
@@ -234,31 +146,14 @@ iptablesAddRemoveRule(iptRules *rules, int action, const 
char *arg, ...)
 
 va_end(args);
 
-if (!(rule = virArgvToString(argv[command_idx])))
-goto error;
-
-if (action == REMOVE) {
-VIR_FREE(argv[command_idx]);
-if (!(argv[command_idx] = strdup(--delete)))
-goto error;
-}
-
 if (virRun(NULL, argv, NULL)  0) {
 retval = errno;
 goto error;
 }
 
-if (action == ADD) {
-retval = iptRulesAppend(rules, rule, argv, command_idx);
-rule = NULL;
-argv = NULL;
-} else {
-retval = iptRulesRemove(rules, rule);
-}
+retval = 0;
 
  error:
-VIR_FREE(rule);
-
 if (argv) {
 n = 0;
 while (argv[n])
@@ -318,50 +213,6 @@ iptablesContextFree(iptablesContext *ctx)
 VIR_FREE(ctx);
 }
 
-static void
-iptRulesReload(iptRules *rules)
-{
-int i;
-char ebuf[1024];
-
-for (i = 0; i  rules-nrules; i++) {
-iptRule *rule = rules-rules[i];
-const char *orig;
-
-orig = rule-argv[rule-command_idx];
-rule-argv[rule-command_idx] = (char *) --delete;
-
-if (virRun(NULL, rule-argv, NULL)  0)
-VIR_WARN(_(Failed to remove iptables rule '%s'
-from chain '%s' in 

[libvirt] [PATCH 5/5] remove now unneeded iptablesContext

[Mark McLoughlin]

iptablesContext no longer contains any state, so we can drop it

* src/util/iptables.c, src/util/iptables.h: drop iptablesContext

* src/network/bridge_driver.c: update callers

* src/libvirt_private.syms: drop context new/free functions
---
 src/libvirt_private.syms|2 -
 src/network/bridge_driver.c |  132 ---
 src/util/iptables.c |  307 ---
 src/util/iptables.h |   59 +++--
 4 files changed, 153 insertions(+), 347 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index e5ba365..d78142e 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -235,8 +235,6 @@ iptablesAddForwardRejectIn;
 iptablesAddForwardRejectOut;
 iptablesAddTcpInput;
 iptablesAddUdpInput;
-iptablesContextFree;
-iptablesContextNew;
 iptablesRemoveForwardAllowCross;
 iptablesRemoveForwardAllowIn;
 iptablesRemoveForwardAllowOut;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index abee78c..28340a1 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -69,7 +69,6 @@ struct network_driver {
 
 virNetworkObjList networks;
 
-iptablesContext *iptables;
 brControl *brctl;
 char *networkConfigDir;
 char *networkAutostartDir;
@@ -247,11 +246,6 @@ networkStartup(int privileged) {
 goto error;
 }
 
-if (!(driverState-iptables = iptablesContextNew())) {
-goto out_of_memory;
-}
-
-
 if (virNetworkLoadAllConfigs(NULL,
  driverState-networks,
  driverState-networkConfigDir,
@@ -349,8 +343,6 @@ networkShutdown(void) {
 
 if (driverState-brctl)
 brShutdown(driverState-brctl);
-if (driverState-iptables)
-iptablesContextFree(driverState-iptables);
 
 networkDriverUnlock(driverState);
 virMutexDestroy(driverState-lock);
@@ -590,13 +582,11 @@ cleanup:
 }
 
 static int
-networkAddMasqueradingIptablesRules(virConnectPtr conn,
-  struct network_driver *driver,
-  virNetworkObjPtr network) {
+networkAddMasqueradingIptablesRules(virConnectPtr conn, virNetworkObjPtr 
network)
+{
 int err;
 /* allow forwarding packets from the bridge interface */
-if ((err = iptablesAddForwardAllowOut(driver-iptables,
-  network-def-network,
+if ((err = iptablesAddForwardAllowOut(network-def-network,
   network-def-bridge,
   network-def-forwardDev))) {
 virReportSystemError(conn, err,
@@ -606,10 +596,9 @@ networkAddMasqueradingIptablesRules(virConnectPtr conn,
 }
 
 /* allow forwarding packets to the bridge interface if they are part of an 
existing connection */
-if ((err = iptablesAddForwardAllowRelatedIn(driver-iptables,
- network-def-network,
- network-def-bridge,
- network-def-forwardDev))) {
+if ((err = iptablesAddForwardAllowRelatedIn(network-def-network,
+network-def-bridge,
+network-def-forwardDev))) {
 virReportSystemError(conn, err,
  _(failed to add iptables rule to allow 
forwarding to '%s'),
  network-def-bridge);
@@ -617,8 +606,7 @@ networkAddMasqueradingIptablesRules(virConnectPtr conn,
 }
 
 /* enable masquerading */
-if ((err = iptablesAddForwardMasquerade(driver-iptables,
-network-def-network,
+if ((err = iptablesAddForwardMasquerade(network-def-network,
 network-def-forwardDev))) {
 virReportSystemError(conn, err,
  _(failed to add iptables rule to enable 
masquerading to '%s'\n),
@@ -629,13 +617,11 @@ networkAddMasqueradingIptablesRules(virConnectPtr conn,
 return 1;
 
  masqerr3:
-iptablesRemoveForwardAllowRelatedIn(driver-iptables,
- network-def-network,
- network-def-bridge,
- network-def-forwardDev);
+iptablesRemoveForwardAllowRelatedIn(network-def-network,
+network-def-bridge,
+network-def-forwardDev);
  masqerr2:
-iptablesRemoveForwardAllowOut(driver-iptables,
-  network-def-network,
+iptablesRemoveForwardAllowOut(network-def-network,
   network-def-bridge,
   network-def-forwardDev);
  masqerr1:
@@ -643,13 +629,11 @@ networkAddMasqueradingIptablesRules(virConnectPtr conn,
 }
 
 static int
-networkAddRoutingIptablesRules(virConnectPtr conn,

Re: [libvirt] [PATCH v2 0/3] Fix migration of paused VMs

[Daniel P. Berrange]

On Wed, Dec 09, 2009 at 02:38:25PM +0100, Paolo Bonzini wrote:
 This is the final version of QEMU support for migrating paused VMs.
 The patch series is the same as what I posted on Nov 25, except for
 the check on the return value of virDomainGetInfo.
 
 Patch 2 also had some easily-solved conflicts upon rebasing.
 
 I haven't yet implemented an error message when inactive domains
 are migrated; I'll do so in a follow-up patch.
 
 Paolo
 
  include/libvirt/libvirt.h.in   |1 +
  src/libvirt.c  |   15 ++-
  src/qemu/qemu_driver.c |   37 -
  src/xen/xend_internal.c|9 +
  tools/virsh.c  |5 -
  tools/virsh.pod|7 ---
  6 files changed, 56 insertions(+), 18 deletions(-)
 
 Paolo Bonzini (3):
   fix migration of paused vms upon failure
   add virsh --suspend
   retrieve paused/running state at the beginning of migration

ACK to this series

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] fc12 virsh tap device problem

[Daniel P. Berrange]

On Tue, Dec 08, 2009 at 07:07:33PM -0800, Chiradeep Vittal wrote:
 I have this xml:
 domain type='kvm'
   namecentos/name
   uuid22d9d573-d82c-c18d-36c0-d3ffef057468/uuid
   memory131072/memory
   vcpu1/vcpu
   os
 type arch='x86_64'hvm/type
   /os
   features
 acpi/
 pae/
   /features
   clock offset='utc'/
   devices
 emulator/usr/bin/qemu-kvm/emulator
 disk type='file' device='disk'
   source file='/var/lib/images/centos.5-4.x86-64/centos-small.img'/
   target dev='hda' bus='ide'/
 /disk
 interface type='user'
   mac address='52:54:00:7e:5b:58'/
 /interface
 interface type='ethernet'
   mac address='52:54:00:2e:33:c8'/
   script path='/var/lib/images/centos.5-4.x86-64/qemu-ifup'/
 /interface

The execution of scripts does not currently work, since we started using 
libcap-ng
to drop all capabilities on QEMU. It will be denied any access to create TAP 
devices
even when running as rot. You need to switch to bridge/network type interfaces 
which
makes libvirt configure the TAP device on QEMU's behalf.

 graphics type='vnc' port='5910' autoport='no' listen=''/
   /devices
 /domain


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Fix compilation failure if yajl is not available

[Daniel P. Berrange]

On Tue, Dec 08, 2009 at 11:13:15AM +0100, Daniel Veillard wrote:
   I commited the following as it was a compile breaker for most people I
 assume and it's trivial
 
 commit a4e09c1ed81f7361ff03d95ad42eac49d4d60812
 Author: Daniel Veillard veill...@redhat.com
 Date:   Tue Dec 8 11:08:17 2009 +0100
 
 Fix a compilation failure if yajl not avail
 
 configure: yajl: no
 CC libvirt_util_la-json.lo
 util/json.c:32:27: error: yajl/yajl_gen.h: No such file or directory
 util/json.c:33:29: error: yajl/yajl_parse.h: No such file or directory
 
 * src/util/json.c: remove the includes if yajl not configured in

Thanks, FYI, I have got yajl in Fedora rawhide, and F12 updates-testing
now if anyone needs it

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] hot swappable HD

[Daniel P. Berrange]

On Mon, Dec 07, 2009 at 03:43:29PM +0100, mbutubuntu wrote:
 hello folks,
 in the real life a SATA Hard Drive is Hot swappable, is there any 
 (non-usb) type of virtual device that is also hot swappable?
 if yes what of ide, scsi, virtio, xen ???

With QEMU/KVM, we support hotplug for VirtIO, SCSI and USB disks

With Xen, we support hotplug for  Xen paravirt disks

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC] New libvirt API for domain memory statistics reporting

[Daniel P. Berrange]

On Tue, Dec 08, 2009 at 02:57:14PM -0500, Adam Litke wrote:
 When using ballooning to manage overcommitted memory on a host, a system for
 guests to communicate their memory usage to the host can provide information
 that will minimize the impact of ballooning on the guests while maximizing
 efficient use of host memory.
 
 The design of such a communication channel was recently added to version 0.8.2
 of the VirtIO Spec (See Appendix G):
  - http://ozlabs.org/~rusty/virtio-spec/virtio-spec-0.8.2.pdf
 
 Host-side and guest-side implementations have been accepted for inclusion in
 their respective projects:
  - Guest: http://lkml.org/lkml/2009/11/30/274
  - Host: http://lists.gnu.org/archive/html/qemu-devel/2009-12/msg00380.html
 
 The following patch series implements a new libvirt interface to expose these
 memory statistics.  Thank you for your review and comments.

I've looked at the code in all the patches, and it all looks good to me.
As others have said the main issue at hand is to get a better definition
of the set of statistics we're going to have in the public struct.

Even though you have included a size_t parameter in the public API, this
does not get usa free ride for the future, because the set of fields is
also part of the RPC wire protocol ABI, and that is not so simple to deal
with. 

This is a going to be quite a popular/useful API for all of our hypervisor
drivers. It definitely  something we can easily implement for VMWare ESX,
OpenVZ, and LXC drivers too. So we should try to get a struct definition
that covers the memory stats required by all

Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] simple LXC/libvirt busybox container (Unable to get cgroup)

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 02:22:37AM -0600, Tony Risinger wrote:
 i'm trying to get even the simplest busybox container with libvirt+LXC
 with very limited success.  I feel l am missing something supremely
 simple for me to be hung on this for weeks.
 
 i dont see anything interesting in domain log, but getting this error
 from LIBVIRT_DEBUG=1 libvirtd:
 
 05:27:56.113: error : lxcDomainGetInfo:462 : internal error Unable to
 get cgroup for arch-nano
 05:27:56.113: debug : virDomainFree:2004 : domain=0x81d8e68
 05:27:56.113: debug : virUnrefDomain:422 : unref domain 0x81d8e68 arch-nano 1
 05:27:56.113: debug : virReleaseDomain:376 : release domain 0x81d8e68 
 arch-nano
 05:27:56.113: debug : virReleaseDomain:392 : unref connection 0x81dc0f0 2
 05:27:56.113: debug : remoteSerializeError:141 : prog=536903814 ver=1
 proc=16 type=1 serial=4, msg=internal error Unable to get cgroup for
 arch-nano
 
 i've been using this root filesystem layout:
 
 [r...@phs-001 arch-nano]# tree
 .
 |-- bin
 |   |-- cat - ../sbin/busybox
 |   |-- chdir - ../sbin/busybox
 |   |-- chmod - ../sbin/busybox
 |   |-- ls - ../sbin/busybox
 |   |-- rm - ../sbin/busybox
 |   |-- sh - ../sbin/busybox
 |   `-- vi - ../sbin/busybox
 |-- dev
 |   `-- pts
 |-- etc
 |-- proc
 |-- sbin
 |   |-- busybox
 |   `-- init - busybox
 `-- sys
 
 all folders besides /bin and /sbin were created by libvirt.  i tried
 using the /sbin/init script previously suggested:
 
 #!/sbin/busybox
 sh


Sorry, my suggestion was wrong. I forgot that if you have #!/sbin/busybox
it will attempt to execute the command matching the name of the script.
So it will in fact try to run 'init', rather than 'sh'. 

Just make the libvirt XML point directly to /bin/sh  instead and it
should work. I even tested it this time :-)

 but i get a similar results either way (script/symlink):
 
  8173 ?Ss 0:00 /usr/lib/libvirt-git/libvirt_lxc --name
 arch-nano --console 11 --background
  8175 pts/0Ss+0:00 init
  8177 ?Ss 0:00 init
  8181 ?Zs 0:00 [init] defunct
  8182 ?Zs 0:00 [init] defunct
  8183 ?Zs 0:00 [init] defunct

Yeah this is what I see too, when i have /sbin/init - changing it to
/bin/sh works


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/5] reload iptables rules simply by re-adding them

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 11:27:51AM +, Mark McLoughlin wrote:
 Currently, when we add iptables rules, we keep them on a list so that
 we can easily reload them on e.g. 'service libvirtd reload'.
 
 However, we don't save this list to disk, so if libvirtd is restarted
 we lose the ability to reload the rules.
 
 The fix is simple - just re-add the damn things on reload.
 
 Note, we delete the rules before re-adding them, just like the current
 behaviour of iptRulesReload().
 
 * src/network/bridge_driver.c: re-add the iptables rules on reload.
 ---
  src/network/bridge_driver.c |   30 --
  1 files changed, 24 insertions(+), 6 deletions(-)
 
 diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
 index 0342aa0..766f8cd 100644
 --- a/src/network/bridge_driver.c
 +++ b/src/network/bridge_driver.c
 @@ -96,6 +96,8 @@ static int networkShutdownNetworkDaemon(virConnectPtr conn,
struct network_driver *driver,
virNetworkObjPtr network);
  
 +static void networkReloadIptablesRules(struct network_driver *driver);
 +
  static struct network_driver *driverState = NULL;
  
  
 @@ -291,12 +293,7 @@ networkReload(void) {
   driverState-networks,
   driverState-networkConfigDir,
   driverState-networkAutostartDir);
 -
 - if (driverState-iptables) {
 -VIR_INFO0(_(Reloading iptables rules\n));
 -iptablesReloadRules(driverState-iptables);
 -}
 -
 +networkReloadIptablesRules(driverState);
  networkAutostartConfigs(driverState);
  networkDriverUnlock(driverState);
  return 0;
 @@ -812,6 +809,27 @@ networkRemoveIptablesRules(struct network_driver *driver,
  iptablesSaveRules(driver-iptables);
  }
  
 +static void
 +networkReloadIptablesRules(struct network_driver *driver)
 +{
 +unsigned int i;
 +
 +VIR_INFO0(_(Reloading iptables rules));
 +
 +for (i = 0 ; i  driver-networks.count ; i++) {
 +virNetworkObjLock(driver-networks.objs[i]);
 +
 +if (virNetworkObjIsActive(driver-networks.objs[i])) {
 +networkRemoveIptablesRules(driver, driver-networks.objs[i]);
 +if (!networkAddIptablesRules(NULL, driver, 
 driver-networks.objs[i])) {
 +/* failed to add but already logged */
 +}
 +}
 +
 +virNetworkObjUnlock(driver-networks.objs[i]);
 +}
 +}
 +
  /* Enable IP Forwarding. Return 0 for success, -1 for failure. */
  static int
  networkEnableIpForwarding(void)

ACK

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/5] reload iptables rules on libvirtd restart

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 11:27:52AM +, Mark McLoughlin wrote:
 This is the expected behaviour, I think - reloading libvirtd should
 be a subset of restarting it.
 
 Note, we reload the rules after we've determined which networks
 are active (because we only add the rules for active networks)
 and before we start autostart networks (to avoid re-adding the
 rules).
 
 * src/network/bridge_driver.c: reload iptables rules on startup
 ---
  src/network/bridge_driver.c |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
 index 766f8cd..d5cab71 100644
 --- a/src/network/bridge_driver.c
 +++ b/src/network/bridge_driver.c
 @@ -259,6 +259,7 @@ networkStartup(int privileged) {
  goto error;
  
  networkFindActiveConfigs(driverState);
 +networkReloadIptablesRules(driverState);
  networkAutostartConfigs(driverState);
  
  networkDriverUnlock(driverState);

ACK


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 3/5] remove all traces of lokkit support

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 11:27:53AM +, Mark McLoughlin wrote:
 Long ago we tried to use Fedora's lokkit utility in order to register
 our iptables rules so that 'service iptables restart' would
 automatically load our rules.
 
 There was one fatal flaw - if the user had configured iptables without
 lokkit, then we would clobber that configuration by running lokkit.
 
 We quickly disabled lokkit support, but never removed it. Let's do
 that now.
 
 The 'my virtual network stops working when I restart iptables' still
 remains. For all the background on this saga, see:
 
   https://bugzilla.redhat.com/227011
 
 * src/util/iptables.c: remove lokkit support
 
 * configure.in: remove --enable-lokkit
 
 * libvirt.spec.in: remove the dirs used only for saving rules for lokkit
 
 * src/Makefile.am: ditto
 
 * src/libvirt_private.syms, src/network/bridge_driver.c,
   src/util/iptables.h: remove references to iptablesSaveRules
 ---
  configure.in|   21 
  libvirt.spec.in |3 -
  src/Makefile.am |4 -
  src/libvirt_private.syms|1 -
  src/network/bridge_driver.c |3 -
  src/util/iptables.c |  218 
 ---
  src/util/iptables.h |1 -
  7 files changed, 0 insertions(+), 251 deletions(-)

ACK, I meant to send this myself in fact.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 4/5] remove iptablesReloadRules() and related code

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 11:27:54AM +, Mark McLoughlin wrote:
 We don't use this method of reloading rules anymore, so we can just
 kill the code.
 
 This simplifies things a lot because we no longer need to keep a
 table of the rules we've added.
 
 * src/util/iptables.c: kill iptablesReloadRules()
 ---
  src/libvirt_private.syms |1 -
  src/util/iptables.c  |  155 
 +-
  src/util/iptables.h  |2 -
  3 files changed, 3 insertions(+), 155 deletions(-)

ACK


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 5/5] remove now unneeded iptablesContext

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 11:27:55AM +, Mark McLoughlin wrote:
 iptablesContext no longer contains any state, so we can drop it
 
 * src/util/iptables.c, src/util/iptables.h: drop iptablesContext
 
 * src/network/bridge_driver.c: update callers
 
 * src/libvirt_private.syms: drop context new/free functions


Ordinarily I'd ACK this, but one of the things I want to try and do
in the future is to move all the libvirt rules out of the main 
INPUT/FORWARD/OUPUT chains, and into sub-chains. I think that the
iptablesContxt struct might be useful for this, so can we leave this
patch out for now.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 5/5] remove now unneeded iptablesContext

[Mark McLoughlin]

On Thu, 2009-12-10 at 12:08 +, Daniel P. Berrange wrote:
 On Thu, Dec 10, 2009 at 11:27:55AM +, Mark McLoughlin wrote:
  iptablesContext no longer contains any state, so we can drop it
  
  * src/util/iptables.c, src/util/iptables.h: drop iptablesContext
  
  * src/network/bridge_driver.c: update callers
  
  * src/libvirt_private.syms: drop context new/free functions
 
 
 Ordinarily I'd ACK this, but one of the things I want to try and do
 in the future is to move all the libvirt rules out of the main 
 INPUT/FORWARD/OUPUT chains, and into sub-chains. I think that the
 iptablesContxt struct might be useful for this, so can we leave this
 patch out for now.

That could done e.g. by using libvirt-INPUT, which again wouldn't need
any state

It's a very nice simplification, easy to re-instate, so I'd prefer to
see it gone rather than for it to stick around under the guise of we
might need it in future. Look how long it took us to delete the lokkit
code after we realized it was useless :)

Cheers,
Mark.

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] simple LXC/libvirt busybox container (Unable to get cgroup)

[Ryota Ozaki]

On Thu, Dec 10, 2009 at 9:03 PM, Daniel P. Berrange berra...@redhat.com wrote:
 On Thu, Dec 10, 2009 at 02:22:37AM -0600, Tony Risinger wrote:
 i'm trying to get even the simplest busybox container with libvirt+LXC
 with very limited success.  I feel l am missing something supremely
 simple for me to be hung on this for weeks.

 i dont see anything interesting in domain log, but getting this error
 from LIBVIRT_DEBUG=1 libvirtd:

 05:27:56.113: error : lxcDomainGetInfo:462 : internal error Unable to
 get cgroup for arch-nano
 05:27:56.113: debug : virDomainFree:2004 : domain=0x81d8e68
 05:27:56.113: debug : virUnrefDomain:422 : unref domain 0x81d8e68 arch-nano 1
 05:27:56.113: debug : virReleaseDomain:376 : release domain 0x81d8e68 
 arch-nano
 05:27:56.113: debug : virReleaseDomain:392 : unref connection 0x81dc0f0 2
 05:27:56.113: debug : remoteSerializeError:141 : prog=536903814 ver=1
 proc=16 type=1 serial=4, msg=internal error Unable to get cgroup for
 arch-nano

 i've been using this root filesystem layout:

 [r...@phs-001 arch-nano]# tree
 .
 |-- bin
 |   |-- cat - ../sbin/busybox
 |   |-- chdir - ../sbin/busybox
 |   |-- chmod - ../sbin/busybox
 |   |-- ls - ../sbin/busybox
 |   |-- rm - ../sbin/busybox
 |   |-- sh - ../sbin/busybox
 |   `-- vi - ../sbin/busybox
 |-- dev
 |   `-- pts
 |-- etc
 |-- proc
 |-- sbin
 |   |-- busybox
 |   `-- init - busybox
 `-- sys

 all folders besides /bin and /sbin were created by libvirt.  i tried
 using the /sbin/init script previously suggested:

 #!/sbin/busybox
 sh


 Sorry, my suggestion was wrong. I forgot that if you have #!/sbin/busybox
 it will attempt to execute the command matching the name of the script.
 So it will in fact try to run 'init', rather than 'sh'.

 Just make the libvirt XML point directly to /bin/sh  instead and it
 should work. I even tested it this time :-)

Hem, I still have a problem with ns subsystem enabled. Yes, I can launch
a container however the cgroup hierarchy is wrong from libvirtd expecting
like:

/: libvirtd --daemon
/5345: /usr/libexec/libvirt_lxc --name

Daniel, could you confirm how about your cgroup hierarchy?

  ozaki-r


 but i get a similar results either way (script/symlink):

  8173 ?        Ss     0:00 /usr/lib/libvirt-git/libvirt_lxc --name
 arch-nano --console 11 --background
  8175 pts/0    Ss+    0:00 init
  8177 ?        Ss     0:00 init
  8181 ?        Zs     0:00 [init] defunct
  8182 ?        Zs     0:00 [init] defunct
  8183 ?        Zs     0:00 [init] defunct

 Yeah this is what I see too, when i have /sbin/init - changing it to
 /bin/sh works


 Daniel
 --
 |: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
 |: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
 |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
 |: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

 --
 Libvir-list mailing list
 Libvir-list@redhat.com
 https://www.redhat.com/mailman/listinfo/libvir-list


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] simple LXC/libvirt busybox container (Unable to get cgroup)

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 09:26:39PM +0900, Ryota Ozaki wrote:
 On Thu, Dec 10, 2009 at 9:03 PM, Daniel P. Berrange berra...@redhat.com 
 wrote:
  On Thu, Dec 10, 2009 at 02:22:37AM -0600, Tony Risinger wrote:
  i'm trying to get even the simplest busybox container with libvirt+LXC
  with very limited success.  I feel l am missing something supremely
  simple for me to be hung on this for weeks.
 
  i dont see anything interesting in domain log, but getting this error
  from LIBVIRT_DEBUG=1 libvirtd:
 
  05:27:56.113: error : lxcDomainGetInfo:462 : internal error Unable to
  get cgroup for arch-nano
  05:27:56.113: debug : virDomainFree:2004 : domain=0x81d8e68
  05:27:56.113: debug : virUnrefDomain:422 : unref domain 0x81d8e68 
  arch-nano 1
  05:27:56.113: debug : virReleaseDomain:376 : release domain 0x81d8e68 
  arch-nano
  05:27:56.113: debug : virReleaseDomain:392 : unref connection 0x81dc0f0 2
  05:27:56.113: debug : remoteSerializeError:141 : prog=536903814 ver=1
  proc=16 type=1 serial=4, msg=internal error Unable to get cgroup for
  arch-nano
 
  i've been using this root filesystem layout:
 
  [r...@phs-001 arch-nano]# tree
  .
  |-- bin
  |   |-- cat - ../sbin/busybox
  |   |-- chdir - ../sbin/busybox
  |   |-- chmod - ../sbin/busybox
  |   |-- ls - ../sbin/busybox
  |   |-- rm - ../sbin/busybox
  |   |-- sh - ../sbin/busybox
  |   `-- vi - ../sbin/busybox
  |-- dev
  |   `-- pts
  |-- etc
  |-- proc
  |-- sbin
  |   |-- busybox
  |   `-- init - busybox
  `-- sys
 
  all folders besides /bin and /sbin were created by libvirt.  i tried
  using the /sbin/init script previously suggested:
 
  #!/sbin/busybox
  sh
 
 
  Sorry, my suggestion was wrong. I forgot that if you have #!/sbin/busybox
  it will attempt to execute the command matching the name of the script.
  So it will in fact try to run 'init', rather than 'sh'.
 
  Just make the libvirt XML point directly to /bin/sh  instead and it
  should work. I even tested it this time :-)
 
 Hem, I still have a problem with ns subsystem enabled. Yes, I can launch
 a container however the cgroup hierarchy is wrong from libvirtd expecting
 like:
 
 /: libvirtd --daemon
 /5345: /usr/libexec/libvirt_lxc --name
 
 Daniel, could you confirm how about your cgroup hierarchy?

What you do mean by 'ns' subsystem ?  


# grep cgroup /proc/mounts 
cgroup /dev/cgroups/cpu cgroup rw,relatime,cpuacct,cpu 0 0
cgroup /dev/cgroups/memory cgroup rw,relatime,memory 0 0
cgroup /dev/cgroups/devices cgroup rw,relatime,devices 0 0

# cat /proc/`pgrep libvirtd`/cgroup
32:devices:/sysdefault
16:memory:/sysdefault
12:cpuacct,cpu:/sysdefault

# cat /proc/`pgrep libvirt_lxc`/cgroup
32:devices:/sysdefault/libvirt/lxc/vm1
16:memory:/sysdefault/libvirt/lxc/vm1
12:cpuacct,cpu:/sysdefault/libvirt/lxc/vm1


And the process inside the contanier is PID 12309

# cat /proc/12309/cgroup 
32:devices:/sysdefault/libvirt/lxc/vm1
16:memory:/sysdefault/libvirt/lxc/vm1
12:cpuacct,cpu:/sysdefault/libvirt/lxc/vm1


Which all appears to be correct to me

This is on a Fedora 12 host 2.6.31.6-145.fc12.i686.PAE with

CONFIG_UTS_NS=y
CONFIG_IPC_NS=y
CONFIG_USER_NS=y
CONFIG_PID_NS=y
CONFIG_NET_NS=y
CONFIG_CGROUP_SCHED=y
CONFIG_CGROUPS=y
# CONFIG_CGROUP_DEBUG is not set
CONFIG_CGROUP_NS=y
CONFIG_CGROUP_FREEZER=y
CONFIG_CGROUP_DEVICE=y
CONFIG_CGROUP_CPUACCT=y
CONFIG_CGROUP_MEM_RES_CTLR=y
CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y
CONFIG_NET_CLS_CGROUP=y


Regards,
Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] simple LXC/libvirt busybox container (Unable to get cgroup)

[Ryota Ozaki]

On Thu, Dec 10, 2009 at 9:36 PM, Daniel P. Berrange berra...@redhat.com wrote:
 On Thu, Dec 10, 2009 at 09:26:39PM +0900, Ryota Ozaki wrote:
 On Thu, Dec 10, 2009 at 9:03 PM, Daniel P. Berrange berra...@redhat.com 
 wrote:
  On Thu, Dec 10, 2009 at 02:22:37AM -0600, Tony Risinger wrote:
  i'm trying to get even the simplest busybox container with libvirt+LXC
  with very limited success.  I feel l am missing something supremely
  simple for me to be hung on this for weeks.
 
  i dont see anything interesting in domain log, but getting this error
  from LIBVIRT_DEBUG=1 libvirtd:
 
  05:27:56.113: error : lxcDomainGetInfo:462 : internal error Unable to
  get cgroup for arch-nano
  05:27:56.113: debug : virDomainFree:2004 : domain=0x81d8e68
  05:27:56.113: debug : virUnrefDomain:422 : unref domain 0x81d8e68 
  arch-nano 1
  05:27:56.113: debug : virReleaseDomain:376 : release domain 0x81d8e68 
  arch-nano
  05:27:56.113: debug : virReleaseDomain:392 : unref connection 0x81dc0f0 2
  05:27:56.113: debug : remoteSerializeError:141 : prog=536903814 ver=1
  proc=16 type=1 serial=4, msg=internal error Unable to get cgroup for
  arch-nano
 
  i've been using this root filesystem layout:
 
  [r...@phs-001 arch-nano]# tree
  .
  |-- bin
  |   |-- cat - ../sbin/busybox
  |   |-- chdir - ../sbin/busybox
  |   |-- chmod - ../sbin/busybox
  |   |-- ls - ../sbin/busybox
  |   |-- rm - ../sbin/busybox
  |   |-- sh - ../sbin/busybox
  |   `-- vi - ../sbin/busybox
  |-- dev
  |   `-- pts
  |-- etc
  |-- proc
  |-- sbin
  |   |-- busybox
  |   `-- init - busybox
  `-- sys
 
  all folders besides /bin and /sbin were created by libvirt.  i tried
  using the /sbin/init script previously suggested:
 
  #!/sbin/busybox
  sh
 
 
  Sorry, my suggestion was wrong. I forgot that if you have #!/sbin/busybox
  it will attempt to execute the command matching the name of the script.
  So it will in fact try to run 'init', rather than 'sh'.
 
  Just make the libvirt XML point directly to /bin/sh  instead and it
  should work. I even tested it this time :-)

 Hem, I still have a problem with ns subsystem enabled. Yes, I can launch
 a container however the cgroup hierarchy is wrong from libvirtd expecting
 like:

 /: libvirtd --daemon
 /5345: /usr/libexec/libvirt_lxc --name

 Daniel, could you confirm how about your cgroup hierarchy?

 What you do mean by 'ns' subsystem ?

'ns' is one of functions of cgroups like such as devices, memory,
cpu, etc. and it is enabled if you mount cgroup without any options
that Tony is doing.



 # grep cgroup /proc/mounts
 cgroup /dev/cgroups/cpu cgroup rw,relatime,cpuacct,cpu 0 0
 cgroup /dev/cgroups/memory cgroup rw,relatime,memory 0 0
 cgroup /dev/cgroups/devices cgroup rw,relatime,devices 0 0

Oh, you don't enable 'ns', so yes, things go fine in your environment.



 # cat /proc/`pgrep libvirtd`/cgroup
 32:devices:/sysdefault
 16:memory:/sysdefault
 12:cpuacct,cpu:/sysdefault

 # cat /proc/`pgrep libvirt_lxc`/cgroup
 32:devices:/sysdefault/libvirt/lxc/vm1
 16:memory:/sysdefault/libvirt/lxc/vm1
 12:cpuacct,cpu:/sysdefault/libvirt/lxc/vm1


 And the process inside the contanier is PID 12309

 # cat /proc/12309/cgroup
 32:devices:/sysdefault/libvirt/lxc/vm1
 16:memory:/sysdefault/libvirt/lxc/vm1
 12:cpuacct,cpu:/sysdefault/libvirt/lxc/vm1


 Which all appears to be correct to me

 This is on a Fedora 12 host 2.6.31.6-145.fc12.i686.PAE with

 CONFIG_UTS_NS=y
 CONFIG_IPC_NS=y
 CONFIG_USER_NS=y
 CONFIG_PID_NS=y
 CONFIG_NET_NS=y
 CONFIG_CGROUP_SCHED=y
 CONFIG_CGROUPS=y
 # CONFIG_CGROUP_DEBUG is not set
 CONFIG_CGROUP_NS=y

This is the function I'm mentioning.

  ozaki-r

 CONFIG_CGROUP_FREEZER=y
 CONFIG_CGROUP_DEVICE=y
 CONFIG_CGROUP_CPUACCT=y
 CONFIG_CGROUP_MEM_RES_CTLR=y
 CONFIG_CGROUP_MEM_RES_CTLR_SWAP=y
 CONFIG_NET_CLS_CGROUP=y


 Regards,
 Daniel
 --
 |: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
 |: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
 |: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
 |: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Re: libvirk KVM save (migrate) terribly slow

[Pierre Riteau]

On 10 déc. 2009, at 12:44, Daniel P. Berrange wrote:

 On Thu, Dec 10, 2009 at 08:05:12AM +0100, Nikola Ciprich wrote:
 Hi,
 I noticed that using libvirt to save running domain is terribly slow,
 I mean it can take even 10 minutes to save VM with 2GB RAM, although
 the host is almost idle (8CPU, 16GB RAM). If I understand correctly, 
 libvirt first pauses the vm and then migrates it to file (optionally through 
 gzip).
 Restoring it back to running state is almost instant.
 I guess this is not what should be expected, is there something particular 
 I should check?
 
 This matches behaviour that I see when saving VMs. QEMU takes a really
 very long time to save itself using migrate exec:.  It is not CPU limited
 even in gzip mode - there is near zero CPU usage while this is going on.
 QEMU just seems to be very slow at sending the data. I've not had time to
 investigate whether this is a flaw in QEMU's exec: migration handling,
 or whether it is being hurt by too small pipe buffers, or something else
 entirely.  I must say I'm not entirely convinced that it is a good idea
 for QEMU to be mixing use of FILE * / popen, with non-blocking I/O, but
 that could be a red-herring.
 
 Daniel

I've reported this issue back when the regression was introduced in qemu.
Anthony had the same idea (not mixing popen with non-blocking I/O), but no 
solution was found at the time.
I haven't tried recently (I'm using TCP migration only) but the problem must 
still be here.

The thread on qemu-devel:
http://lists.gnu.org/archive/html/qemu-devel/2009-08/msg01557.html
http://lists.gnu.org/archive/html/qemu-devel/2009-09/msg00020.html

-- 
Pierre Riteau -- http://perso.univ-rennes1.fr/pierre.riteau/


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/3] Make QEMU driver use -chardev everywhere when it's available

[Matthew Booth]

Change -monitor, -serial and -parallel output to use -chardev if it is
available.

* src/qemu/qemu_conf.c: Update qemudBuildCommandLine to use -chardev where
  available.
* tests/qemuxml2argvtest.c tests/qemuxml2argvdata/: Add -chardev equivalents for
  all current serial and parallel tests.
---
 src/qemu/qemu_conf.c   |  108 +---
 .../qemuxml2argv-channel-guestfwd.args |2 +-
 .../qemuxml2argv-console-compat-chardev.args   |1 +
 .../qemuxml2argv-console-compat-chardev.xml|   28 +
 .../qemuxml2argv-parallel-tcp-chardev.args |1 +
 .../qemuxml2argv-parallel-tcp-chardev.xml  |   27 +
 .../qemuxml2argv-serial-dev-chardev.args   |1 +
 .../qemuxml2argv-serial-dev-chardev.xml|   30 ++
 .../qemuxml2argv-serial-file-chardev.args  |1 +
 .../qemuxml2argv-serial-file-chardev.xml   |   30 ++
 .../qemuxml2argv-serial-many-chardev.args  |1 +
 .../qemuxml2argv-serial-many-chardev.xml   |   32 ++
 .../qemuxml2argv-serial-pty-chardev.args   |1 +
 .../qemuxml2argv-serial-pty-chardev.xml|   28 +
 .../qemuxml2argv-serial-tcp-chardev.args   |1 +
 .../qemuxml2argv-serial-tcp-chardev.xml|   32 ++
 .../qemuxml2argv-serial-tcp-telnet-chardev.args|1 +
 .../qemuxml2argv-serial-tcp-telnet-chardev.xml |   32 ++
 .../qemuxml2argv-serial-udp-chardev.args   |1 +
 .../qemuxml2argv-serial-udp-chardev.xml|   32 ++
 .../qemuxml2argv-serial-unix-chardev.args  |1 +
 .../qemuxml2argv-serial-unix-chardev.xml   |   30 ++
 .../qemuxml2argv-serial-vc-chardev.args|1 +
 .../qemuxml2argv-serial-vc-chardev.xml |   28 +
 tests/qemuxml2argvtest.c   |   12 ++
 25 files changed, 445 insertions(+), 17 deletions(-)
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-console-compat-chardev.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-console-compat-chardev.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp-chardev.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-parallel-tcp-chardev.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-dev-chardev.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-file-chardev.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-file-chardev.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-many-chardev.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-many-chardev.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-pty-chardev.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-chardev.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-chardev.xml
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet-chardev.args
 create mode 100644 
tests/qemuxml2argvdata/qemuxml2argv-serial-tcp-telnet-chardev.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-udp-chardev.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-udp-chardev.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-unix-chardev.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-unix-chardev.xml
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-vc-chardev.args
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-serial-vc-chardev.xml

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index a25fac6..86172c6 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1863,17 +1863,37 @@ int qemudBuildCommandLine(virConnectPtr conn,
 if (monitor_chr) {
 virBuffer buf = VIR_BUFFER_INITIALIZER;
 
-if (monitor_json)
-virBufferAddLit(buf, control,);
+/* Use -chardev if it's available */
+if (qemuCmdFlags  QEMUD_CMD_FLAG_CHARDEV) {
+qemudBuildCommandLineChrDevChardevStr(monitor_chr, monitor, 
buf);
+if (virBufferError(buf)) {
+virBufferFreeAndReset(buf);
+goto no_memory;
+}
+
+ADD_ARG_LIT(-chardev);
+ADD_ARG(virBufferContentAndReset(buf));
+
+if (monitor_json)
+virBufferAddLit(buf, control,);
+
+virBufferAddLit(buf, chardev:monitor);
+}
+
+else {
+if (monitor_json)
+virBufferAddLit(buf, control,);
+
+qemudBuildCommandLineChrDevStr(monitor_chr, buf);
+}
 
-qemudBuildCommandLineChrDevStr(monitor_chr, buf);
 if (virBufferError(buf)) {
 virBufferFreeAndReset(buf);
 goto no_memory;
 }
 

[libvirt] [PATCH 2/3] Extract the assigned pty device for channels

[Matthew Booth]

* src/qemu/qemu_driver.c: Parse pty devices for channels
---
 src/qemu/qemu_driver.c |   10 ++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2fb059d..c1feb0f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1439,6 +1439,16 @@ qemudFindCharDevicePTYs(virConnectPtr conn,
 }
 }
 
+/* then the channel devices */
+for (i = 0 ; i  vm-def-nchannels ; i++) {
+virDomainChrDefPtr chr = vm-def-channels[i];
+if (chr-type == VIR_DOMAIN_CHR_TYPE_PTY) {
+if ((ret = qemudExtractTTYPath(conn, output, offset,
+   chr-data.file.path)) != 0)
+return ret;
+}
+}
+
 return 0;
 }
 
-- 
1.6.5.2

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 3/3] Get QEMU pty paths from the monitor

[Matthew Booth]

This change makes the QEMU driver get pty paths from the output of the monitor
'info chardev' command. This output is structured, and contains both the name of
the device and the path on the same line. This is considerably more reliable
than parsing the startup log output, which requires the parsing code to know
which order QEMU will print pty information in.

Note that we still need to parse the log output as the monitor itself may be on
a pty. This should be rare, however, and the new code will replace all pty paths
parsed by the log output method once the monitor is available.

* src/qemu/qemu_monitor.(c|h) src/qemu_monitor_text.(c|h): Implement
  qemuMonitorGetPtyPaths().
* src/qemu/qemu_driver.c: Get pty path information using
  qemuMonitorGetPtyPaths().
---
 src/qemu/qemu_driver.c   |   68 +++-
 src/qemu/qemu_monitor.c  |9 +
 src/qemu/qemu_monitor.h  |3 ++
 src/qemu/qemu_monitor_text.c |   71 ++
 src/qemu/qemu_monitor_text.h |4 ++
 5 files changed, 153 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c1feb0f..6f1fc1a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1407,6 +1407,40 @@ qemudExtractTTYPath(virConnectPtr conn,
 }
 
 static int
+qemudFindCharDevicePTYsMonitor(virConnectPtr conn,
+   virDomainObjPtr vm,
+   virHashTablePtr paths)
+{
+int i;
+
+#define LOOKUP_PTYS(array, arraylen, idprefix) \
+for (i = 0 ; i  (arraylen) ; i++) { \
+virDomainChrDefPtr chr = (array)[i]; \
+if (chr-type == VIR_DOMAIN_CHR_TYPE_PTY) { \
+char id[16]; \
+\
+if (snprintf(id, sizeof(id), idprefix %i, i) = sizeof(id)) \
+return -1; \
+\
+const char *path = (const char *) virHashLookup(paths, id); \
+if (path == NULL) { \
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, \
+ _(no assigned pty for device %s), id); \
+return -1; \
+} \
+\
+chr-data.file.path = strdup(path); \
+} \
+}
+
+LOOKUP_PTYS(vm-def-serials,   vm-def-nserials,   serial);
+LOOKUP_PTYS(vm-def-parallels, vm-def-nparallels, parallel);
+LOOKUP_PTYS(vm-def-channels,  vm-def-nchannels,  channel);
+
+return 0;
+}
+
+static int
 qemudFindCharDevicePTYs(virConnectPtr conn,
 virDomainObjPtr vm,
 const char *output,
@@ -1452,6 +1486,11 @@ qemudFindCharDevicePTYs(virConnectPtr conn,
 return 0;
 }
 
+static void qemudFreePtyPath(void *payload, const char *name ATTRIBUTE_UNUSED)
+{
+free(payload);
+}
+
 static int
 qemudWaitForMonitor(virConnectPtr conn,
 struct qemud_driver* driver,
@@ -1459,7 +1498,7 @@ qemudWaitForMonitor(virConnectPtr conn,
 {
 char buf[4096]; /* Plenty of space to get startup greeting */
 int logfd;
-int ret;
+int ret = -1;
 
 if ((logfd = qemudLogReadFD(conn, driver-logDir, vm-def-name, pos))
  0)
@@ -1485,7 +1524,32 @@ qemudWaitForMonitor(virConnectPtr conn,
 if (qemuConnectMonitor(vm)  0)
 return -1;
 
-return 0;
+/* Try to get the pty path mappings again via the monitor. This is much 
more
+ * reliable if it's available.
+ * Note that the monitor itself can be on a pty, so we still need to try 
the
+ * log output method. */
+virHashTablePtr paths = virHashCreate(0);
+if (paths == NULL) {
+virReportOOMError(NULL);
+goto cleanup;
+}
+
+qemuDomainObjEnterMonitor(vm);
+qemuDomainObjPrivatePtr priv = vm-privateData;
+ret = qemuMonitorGetPtyPaths(priv-mon, paths);
+qemuDomainObjExitMonitor(vm);
+
+VIR_DEBUG(qemuMonitorGetPtyPaths returned %i, ret);
+if (ret == 0) {
+ret = qemudFindCharDevicePTYsMonitor(conn, vm, paths);
+}
+
+cleanup:
+if (paths) {
+virHashFree(paths, qemudFreePtyPath);
+}
+
+return ret;
 }
 
 static int
diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 103cf28..750e3e6 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -1267,3 +1267,12 @@ int qemuMonitorRemoveHostNetwork(qemuMonitorPtr mon,
 ret = qemuMonitorTextRemoveHostNetwork(mon, vlan, netname);
 return ret;
 }
+
+int qemuMonitorGetPtyPaths(qemuMonitorPtr mon,
+   virHashTablePtr paths)
+{
+DEBUG(mon=%p, fd=%d,
+  mon, mon-fd);
+
+return qemuMonitorTextGetPtyPaths(mon, paths);
+}
diff --git a/src/qemu/qemu_monitor.h b/src/qemu/qemu_monitor.h
index 8b1e3a3..db00b26 100644
--- a/src/qemu/qemu_monitor.h
+++ b/src/qemu/qemu_monitor.h
@@ -28,6 +28,7 @@
 #include internal.h
 
 #include domain_conf.h
+#include hash.h
 
 typedef struct _qemuMonitor qemuMonitor;
 typedef qemuMonitor *qemuMonitorPtr;
@@ -272,5 +273,7 @@ int 

Re: [libvirt] [RFC] New libvirt API for domain memory statistics reporting

[Adam Litke]

On Thu, 2009-12-10 at 11:54 +, Daniel P. Berrange wrote:
 I've looked at the code in all the patches, and it all looks good to me.
 As others have said the main issue at hand is to get a better definition
 of the set of statistics we're going to have in the public struct.
 
 Even though you have included a size_t parameter in the public API, this
 does not get usa free ride for the future, because the set of fields is
 also part of the RPC wire protocol ABI, and that is not so simple to deal
 with. 
 
 This is a going to be quite a popular/useful API for all of our hypervisor
 drivers. It definitely  something we can easily implement for VMWare ESX,
 OpenVZ, and LXC drivers too. So we should try to get a struct definition
 that covers the memory stats required by all

Thanks for your review.  I am working on an alternative way of
marshaling the statistics.  I will work on providing more concise
definitions of each current statistic as well. 

-- 
Thanks,
Adam

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Fix error in secret.rng schema.

[Diego Elio 'Flameeyes' Pettenò]

With this error the schema is not a well-formed XML file.
---
 docs/schemas/secret.rng |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng
index 40e2b7f..39f8625 100644
--- a/docs/schemas/secret.rng
+++ b/docs/schemas/secret.rng
@@ -36,7 +36,7 @@
 optional
   element name='usage'
 choice
-  ref name='usagevolume'
+  ref name='usagevolume' /
   !-- More choices later --
 /choice
   /element
-- 
1.6.5.5

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] Apologies for miss-configured Out-of-Office auto-replies

[Konrad Eriksson1]

Dear list members,

I'm terribly sorry for my badly configured Out-of-Office auto-reply 
feature in my email client that might have caused several replies to 
libvir-list emails received by me.

It should be solved now (thanks to Daniel Veillard for pointing it out).


Again sorry for the unintentional spaming.

Happy virtualization!

Regards,
Konrad Eriksson--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Fix error in secret.rng schema.

[Diego Elio 'Flameeyes' Pettenò]

Without this error the schema is not a well-formatted XML file.
---
 docs/schemas/secret.rng |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng
index 40e2b7f..39f8625 100644
--- a/docs/schemas/secret.rng
+++ b/docs/schemas/secret.rng
@@ -36,7 +36,7 @@
 optional
   element name='usage'
 choice
-  ref name='usagevolume'
+  ref name='usagevolume' /
   !-- More choices later --
 /choice
   /element
-- 
1.6.5.5

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] My beefs with the libvirt XML configuration format

[Diego Elio “Flameeyes” Pettenò]

Hello,

In a recent post on my blog [1] I ranted on about libvirt and in
particular I complained that the configuration files look like what I
call “almost XML”. The reasons why I say that are multiple, let me try
to explain some.

In the configuration files, at least those created by virt-manager there
is no specification of what the file should be (no document type, no
namespace, and, IMHO, a too generic root element name); given that some
kind of distinction is needed for software like Emacs's nxml-mode to
know how to deal with the file, I think that's pretty bad for
interaction between different applications. While libvirt knows
perfectly well what it's dealing with, other packages might not. Might
not sound a major issue but it starts tickling my senses when this
happens.

The configuration seem somewhat contrived in places like the disk
configuration: if the disk is file-backed it require the file attribute
to the source element, while it needs the dev attribute if it's a
block device; given that it's a path in both cases it would have sounded
easier on the user if a single path attribute was used. But this is
opinable.

The third problem I called out for in the block is a lack of a schema
for the files; Daniel corrected me pointing out that the schemas are
distributed with the sources and installed. Sure thing, I was wrong. On
the other hand I maintain that there are problems with those schemas.
The first is that both the version distributed with 0.7.4 and the git
version as of today suffer from bug #546254 [2] (secret.rng being not
well formed) so it means nobody has even tested them as of lately; then
there is the fact that they are never referenced by the human-readable
documentation [3] which is why I didn't find it the first time around;
add also to that some contrived syntax in those schema as well that
causes trang to produce a non-valid rnc file out of them (nxml-mode uses
rnc rather than rng).

But I guess the one big problem with the schemas is that they don't seem
to properly encode what the human-readable documentation says, or what
virt-manager does. For instance (please follow me with selector-like
syntax), virt-manager creates /domain/os/ty...@machine='pc-0.11'] in the
created XML; the same attribute seem to be documented: “There are also
two optional attributes, arch  specifying the CPU architecture to
virtualization, and machine  referring to the machine type”. The schema
does not seem to accept that attribute though (“element type: Relax-NG
validity error : Invalid attribute machine for element type” with
xmllint, just to make sure that it's not a bug in any other piece of
software, this is Daniel's libxml2).

Now after voicing my opinions here, as Daniel dared me to do, I'd like
to explain a second why I didn't post this on the list in the first
place: of what I wrote here, my beefs for calling this aXML, the only
things that can be solved easily are the schemas; schemas that, at the
time I wrote the blog, I was unable to find. The syntax, and the lack of
a “safe” identification of the files as libvirt's are the kind of legacy
problems one has to deal with to avoid wasting users' time with
migrations and corrections, so I don't really think they should be
addressed unless a redesign of the configuration is intended.

Just my two cents, you're free to take them as you wish, I cannot boast
a curriculum like Daniel's, but I don't think I'm stepping out of place
to point out these things.

[1]
http://blog.flameeyes.eu/2009/12/07/i-know-you-missed-them-virtualisation-ranting-goes-on
[2] https://bugzilla.redhat.com/show_bug.cgi?id=546254
[3] http://libvirt.org/format.html

-- 
Diego Elio Pettenò — “Flameeyes”
http://blog.flameeyes.eu/

If you found a .asc file in this mail and know not what it is,
it's a GnuPG digital signature: http://www.gnupg.org/


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Fix error in secret.rng schema.

[Daniel Veillard]

On Thu, Dec 10, 2009 at 03:57:12PM +0100, Diego Elio 'Flameeyes' Pettenò wrote:
 Without this error the schema is not a well-formatted XML file.
 ---
  docs/schemas/secret.rng |2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)
 
 diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng
 index 40e2b7f..39f8625 100644
 --- a/docs/schemas/secret.rng
 +++ b/docs/schemas/secret.rng
 @@ -36,7 +36,7 @@
  optional
element name='usage'
  choice
 -  ref name='usagevolume'
 +  ref name='usagevolume' /
!-- More choices later --
  /choice
/element

  Whoops, good catch, apparently we are missing regression tests for
the secret XML validation ... !

  thanks ! Pushed

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Replace optionaloneOrMore with zeroOrMore.

[Diego Elio 'Flameeyes' Pettenò]

The two syntax should be equivalent, but the former creates, with trang, an
invalid Relax-NG Compact form file, as the output will contain the sequence
+? which is not recognized by tools like rnv.
---
 docs/schemas/domain.rng |   40 +++-
 1 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
index 5b8f7f8..d1d3efb 100644
--- a/docs/schemas/domain.rng
+++ b/docs/schemas/domain.rng
@@ -980,27 +980,25 @@
 /attribute
   /define
   define name=qemucdevSrcDef
-optional
-  oneOrMore
-element name=source
-  optional
-attribute name=mode/
-  /optional
-  optional
-attribute name=path/
-  /optional
-  optional
-attribute name=host/
-  /optional
-  optional
-attribute name=service/
-  /optional
-  optional
-attribute name=wiremode/
-  /optional
-/element
-  /oneOrMore
-/optional
+zeroOrMore
+  element name=source
+optional
+  attribute name=mode/
+/optional
+optional
+  attribute name=path/
+/optional
+optional
+  attribute name=host/
+/optional
+optional
+  attribute name=service/
+/optional
+optional
+  attribute name=wiremode/
+/optional
+  /element
+/zeroOrMore
 optional
   element name=protocol
 optional
-- 
1.6.5.5

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Segmentation fault when shutting down VMs

[Thomas Treutner]

On Friday 13 November 2009 12:46:24 Daniel P. Berrange wrote:
 On Fri, Nov 13, 2009 at 11:42:46AM +0100, Thomas Treutner wrote:
  Hi,
 
  I'm facing another problem with latest git.
 
  This happens regularly when shutting down VM(s):
 
  http://pastebin.com/m4ef1d93f
 
  It doesn't depend on which or how many VM is/are shut down, or if I have
  a VNC connection to a VM that is shutting down. Sometimes after a segv, I
  get this:
 
  http://pastebin.com/m5260bf21
 
  It seems to me that libvirt tries to query a qemu monitor that is no
  longer alive?

 This second problem is fixed by

 http://www.redhat.com/archives/libvir-list/2009-November/msg00458.html


 I'm still trying to figure out what's going on with the first problem

I found some time to pull latest git, build, and test a little bit today, and 
it seems to me that the problem has vanished away!


-t

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] qemu driver: Fix segfault in libvirt/libvirtd when uri-path is NULL

[Richard W.M. Jones]

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v
From c11a82b62aefc21e070c527f59a1f9c57a7b4f36 Mon Sep 17 00:00:00 2001
From: Richard Jones rjo...@redhat.com
Date: Thu, 10 Dec 2009 16:39:07 +
Subject: [PATCH] qemu driver: Fix segfault in libvirt/libvirtd when uri-path 
is NULL.

See also:
https://bugzilla.redhat.com/show_bug.cgi?id=545400#c1
---
 src/qemu/qemu_driver.c |9 +
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2fb059d..e9cc8c3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2651,6 +2651,15 @@ static virDrvOpenStatus qemudOpen(virConnectPtr conn,
 return VIR_DRV_OPEN_ERROR;
 }
 
+if (conn-uri-path == NULL) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(no QEMU URI path given, try %s),
+ qemu_driver-privileged
+   ? qemu:///system
+   : qemu:///session);
+return VIR_DRV_OPEN_ERROR;
+}
+
 if (qemu_driver-privileged) {
 if (STRNEQ (conn-uri-path, /system) 
 STRNEQ (conn-uri-path, /session)) {
-- 
1.6.5.2

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Segmentation fault when shutting down VMs

[Matthias Bolte]

2009/12/10 Thomas Treutner tho...@scripty.at:
 On Friday 13 November 2009 12:46:24 Daniel P. Berrange wrote:
 On Fri, Nov 13, 2009 at 11:42:46AM +0100, Thomas Treutner wrote:
  Hi,
 
  I'm facing another problem with latest git.
 
  This happens regularly when shutting down VM(s):
 
  http://pastebin.com/m4ef1d93f
 
  It doesn't depend on which or how many VM is/are shut down, or if I have
  a VNC connection to a VM that is shutting down. Sometimes after a segv, I
  get this:
 
  http://pastebin.com/m5260bf21
 
  It seems to me that libvirt tries to query a qemu monitor that is no
  longer alive?

 This second problem is fixed by

 http://www.redhat.com/archives/libvir-list/2009-November/msg00458.html


 I'm still trying to figure out what's going on with the first problem

 I found some time to pull latest git, build, and test a little bit today, and
 it seems to me that the problem has vanished away!


 -t


The problem has been fixed by the combination of this two commits:

http://libvirt.org/git/?p=libvirt.git;a=commit;h=79533da1b36cc16e0f3f8aea798d6cc20528c451
http://libvirt.org/git/?p=libvirt.git;a=commit;h=8e7d14953ca6ef047112bc6ac177507ea8f824f2

Matthias

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] My beefs with the libvirt XML configuration format

[Daniel Veillard]

On Thu, Dec 10, 2009 at 04:51:54PM +0100, Diego Elio “Flameeyes” Pettenò wrote:
 Hello,
 
 In a recent post on my blog [1] I ranted on about libvirt and in
 particular I complained that the configuration files look like what I
 call “almost XML”. The reasons why I say that are multiple, let me try
 to explain some.
 
 In the configuration files, at least those created by virt-manager there
 is no specification of what the file should be (no document type, no
 namespace, and, IMHO, a too generic root element name); given that some

  Okay, I have been in the standardization space for XML for a decade
now, so I have an opinion on those issues :-)

  Document type: DTDs are obsolete, the level of checking they provide
 is really not really useful.
  Namespace: in that case that would be combersome, namespaces are
  useful when you want to mix vocabularies coming from different
  places. In our use case I don't see any need to mix, the format
  is driven by an application. Major drawback if we had used
  namespaces, even a default one, all the XPath expressions and
  queries we are doing in the code would then need a namespace
  registration and a prefix, this is painful for just no gain in our
  case.
  Generic element name: well we used what was the most sensible based on
  the vocabulary used in http://libvirt.org/goals.html the goal
  documentation

 kind of distinction is needed for software like Emacs's nxml-mode to
 know how to deal with the file, I think that's pretty bad for
 interaction between different applications. While libvirt knows

  Your viewpoint is that users should edit the XML. My viewpoint is that
software should do this, basically in normal use of libvirt nobody
should have to look at the XML, the virt-viewer/virt-install etc...
tools should generate and handle those for you. It happens that one may
have to tweak something like a pathname in a definition or something but
whatever the level of schemas available it won't help for that kind of
tweaks. Things like changing the ethernet type adapter should be a
pull down list in a gui like virt-manager, not loading the saved xml in
emacs, finding the associated relax-ng, finding the place where it's
defined and hoping emacs will get a list to pick from, which even if you
did it right might just not work because the domain is running and
your change would be discarded on restart... oops.

 perfectly well what it's dealing with, other packages might not. Might
 not sound a major issue but it starts tickling my senses when this
 happens.
 
 The configuration seem somewhat contrived in places like the disk
 configuration: if the disk is file-backed it require the file attribute
 to the source element, while it needs the dev attribute if it's a
 block device; given that it's a path in both cases it would have sounded
 easier on the user if a single path attribute was used. But this is
 opinable.

  Yup and there have been opinions. The good point are that all the
pros and cons which led to the format definition (and I tell you we got
much discussion on format issues !) are all documented in the mailing
list archives so you can find why we choose such or such format,
sometimes we have remains from history, but honnestly the format is
not that bad considering the heavy use, the incredible variations from
hypervisors to hypervisors and the few years of history !

 The third problem I called out for in the block is a lack of a schema
 for the files; Daniel corrected me pointing out that the schemas are
 distributed with the sources and installed. Sure thing, I was wrong. On
 the other hand I maintain that there are problems with those schemas.
 The first is that both the version distributed with 0.7.4 and the git
 version as of today suffer from bug #546254 [2] (secret.rng being not
 well formed) so it means nobody has even tested them as of lately; then

  As you pointed out, this is now fixed in GIT, the secret handling was
added recently and apparently we didn't add regresison testing, we
usually use the Relax-NG for validating example in make check

 there is the fact that they are never referenced by the human-readable
 documentation [3] which is why I didn't find it the first time around;

  There is a tool /usr/bin/virt-xml-validate on my system part of
libvirt-client package which does the va;idation for domain file, we
don't expect people to know RNG or know how to validate against a .rng
The format is documented

 add also to that some contrived syntax in those schema as well that
 causes trang to produce a non-valid rnc file out of them (nxml-mode uses
 rnc rather than rng).

  Ah, well this sounds like a bug in trang, you could try to reach James
Clark for fixing, I though there was formal equivalence in his compact
syntax. In any case the XML syntax is the core ISO spec, and I find
normal to use that one.

 But I guess the one big problem with the schemas is that they don't seem
 to properly 

Re: [libvirt] [PATCH] qemu driver: Fix segfault in libvirt/libvirtd when uri-path is NULL

[Daniel Veillard]

On Thu, Dec 10, 2009 at 04:40:14PM +, Richard W.M. Jones wrote:
 
 -- 
 Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
 virt-p2v converts physical machines to virtual machines.  Boot with a
 live CD or over the network (PXE) and turn machines into Xen guests.
 http://et.redhat.com/~rjones/virt-p2v

 From c11a82b62aefc21e070c527f59a1f9c57a7b4f36 Mon Sep 17 00:00:00 2001
 From: Richard Jones rjo...@redhat.com
 Date: Thu, 10 Dec 2009 16:39:07 +
 Subject: [PATCH] qemu driver: Fix segfault in libvirt/libvirtd when uri-path 
 is NULL.
 
 See also:
 https://bugzilla.redhat.com/show_bug.cgi?id=545400#c1
 ---
  src/qemu/qemu_driver.c |9 +
  1 files changed, 9 insertions(+), 0 deletions(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 2fb059d..e9cc8c3 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -2651,6 +2651,15 @@ static virDrvOpenStatus qemudOpen(virConnectPtr conn,
  return VIR_DRV_OPEN_ERROR;
  }
  
 +if (conn-uri-path == NULL) {
 +qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
 + _(no QEMU URI path given, try %s),
 + qemu_driver-privileged
 +   ? qemu:///system
 +   : qemu:///session);
 +return VIR_DRV_OPEN_ERROR;
 +}
 +
  if (qemu_driver-privileged) {
  if (STRNEQ (conn-uri-path, /system) 
  STRNEQ (conn-uri-path, /session)) {

  Whoops, and that's easy to get wrong, ACK, please push !

   thanks !

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] My beefs with the libvirt XML configuration format

[Cole Robinson]

On 12/10/2009 10:51 AM, Diego Elio “Flameeyes” Pettenò wrote:
 Hello,
 
 In a recent post on my blog [1] I ranted on about libvirt and in
 particular I complained that the configuration files look like what I
 call “almost XML�. The reasons why I say that are multiple, let me try
 to explain some.
 
 In the configuration files, at least those created by virt-manager there
 is no specification of what the file should be (no document type, no
 namespace, and, IMHO, a too generic root element name); given that some
 kind of distinction is needed for software like Emacs's nxml-mode to
 know how to deal with the file, I think that's pretty bad for
 interaction between different applications. While libvirt knows
 perfectly well what it's dealing with, other packages might not. Might
 not sound a major issue but it starts tickling my senses when this
 happens.
 
 The configuration seem somewhat contrived in places like the disk
 configuration: if the disk is file-backed it require the file attribute
 to the source element, while it needs the dev attribute if it's a
 block device; given that it's a path in both cases it would have sounded
 easier on the user if a single path attribute was used. But this is
 opinable.
 

This is something that has always bugged me as well, and is indeed a
pain to deal with in virt-manager when doing things like changing cdrom
media. I think we should just loosen the input restrictions, so that any
path passed via the source properties file, dev, or dir are used, but
will be dumped with the 'correct' property to maintain back compat.

That said, I think the XML format is pretty straight forward. The above
caveat you mention is the only real annoying thing. The one other
stumbling block is that not all devices have a unique property to key
off of in the XML (ex. you can have two identical video devices). This
makes life difficult for virt-manager when removing a device, but it
hasn't been a big issue in practice.

 The third problem I called out for in the block is a lack of a schema
 for the files; Daniel corrected me pointing out that the schemas are
 distributed with the sources and installed. Sure thing, I was wrong. On
 the other hand I maintain that there are problems with those schemas.
 The first is that both the version distributed with 0.7.4 and the git
 version as of today suffer from bug #546254 [2] (secret.rng being not
 well formed) so it means nobody has even tested them as of lately; then
 there is the fact that they are never referenced by the human-readable
 documentation [3] which is why I didn't find it the first time around;
 add also to that some contrived syntax in those schema as well that
 causes trang to produce a non-valid rnc file out of them (nxml-mode uses
 rnc rather than rng).
 

The bug you mention only exists because we don't have secret XML
regression tests. Other schemas are in better shape: I'm pretty
confident that virtual network and storage pool/volume schemas have near
complete coverage. Domain probably has very high coverage but there are
no doubt nuances of the format that aren't covered by our regression
suite, and therefor may have incorrect schemas.

For a long while we didn't use the XML schemas for anything so they were
horrendously out of date and practically useless. That has changed
within the past year but we are still playing catchup to have the schema
match libvirt code reality.

Putting a link to schemas on the website is also a good idea.

 But I guess the one big problem with the schemas is that they don't seem
 to properly encode what the human-readable documentation says, or what
 virt-manager does. For instance (please follow me with selector-like
 syntax), virt-manager creates /domain/os/ty...@machine='pc-0.11'] in the
 created XML; the same attribute seem to be documented: “There are also
 two optional attributes, arch  specifying the CPU architecture to
 virtualization, and machine  referring to the machine type�. The schema
 does not seem to accept that attribute though (“element type: Relax-NG
 validity error : Invalid attribute machine for element type� with
 xmllint, just to make sure that it's not a bug in any other piece of
 software, this is Daniel's libxml2).
 

If there are any other pieces of the schema that you find are incorrect
or don't match reality, please enumerate them here and I'll take some
time to make sure they are tested in our regression suite.

Thanks,
Cole

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] My beefs with the libvirt XML configuration format

[Matthias Bolte]

2009/12/10 Daniel Veillard veill...@redhat.com:

  So thanks for reporting the existing bugs, I ended up on your blog
 post randomly and I though the best was to get you here to actually
 solve the issues, seems to have worked better than I expected :-)
 There is still that crash problem you wrote about and which would need
 some clarification, Im' not sure I really understood what went wrong !

 Daniel


The crash could be due to the QEMU monitor handling problem that was
recently fixed. The problem was that the asynchronous monitor handling
code would free the monitor object in case QEMU crashed or reported an
error. But the asynchronous monitor handling freed the monitor object
while the caller was not aware of this and the caller then accessed
freed memory resulting in a crash.

Matthias

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] My beefs with the libvirt XML configuration format

[Diego Elio “Flameeyes” Pettenò]

Il giorno Thu, 10/12/2009 alle 18.28 +0100, Matthias Bolte ha scritto:
 
 The crash could be due to the QEMU monitor handling problem that was
 recently fixed.

Yup that seems to be it, tied with a crash in qemu-kvm's 0.11.0 handling
of virtio network (qemu crashed, which then caused libvirt to crash
again) which is solved in 0.11.1.

I'm sincerely not sure at this point if I should wait for 0.7.5 or look
for a way to backport the needed change.

-- 
Diego Elio Pettenò — “Flameeyes”
http://blog.flameeyes.eu/

If you found a .asc file in this mail and know not what it is,
it's a GnuPG digital signature: http://www.gnupg.org/


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] My beefs with the libvirt XML configuration format

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 12:09:23PM -0500, Cole Robinson wrote:
 
 This is something that has always bugged me as well, and is indeed a
 pain to deal with in virt-manager when doing things like changing cdrom
 media. I think we should just loosen the input restrictions, so that any
 path passed via the source properties file, dev, or dir are used, but
 will be dumped with the 'correct' property to maintain back compat.
 
 That said, I think the XML format is pretty straight forward. The above
 caveat you mention is the only real annoying thing. The one other
 stumbling block is that not all devices have a unique property to key
 off of in the XML (ex. you can have two identical video devices). This
 makes life difficult for virt-manager when removing a device, but it
 hasn't been a big issue in practice.

That is going to change in the near future. I'm about to post patches
which give every single device a address element containing their
unique PCI, USB, or disk controller address. In addition every device
will likely get a unique short name.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] My beefs with the libvirt XML configuration format

[Richard W.M. Jones]

On Thu, Dec 10, 2009 at 05:44:43PM +0100, Daniel Veillard wrote:
   Your viewpoint is that users should edit the XML. My viewpoint is that
 software should do this, basically in normal use of libvirt nobody
 should have to look at the XML, the virt-viewer/virt-install etc...
 tools should generate and handle those for you. It happens that one may
 have to tweak something like a pathname in a definition or something but
 whatever the level of schemas available it won't help for that kind of
 tweaks. Things like changing the ethernet type adapter should be a
 pull down list in a gui like virt-manager, not loading the saved xml in
 emacs, finding the associated relax-ng, finding the place where it's
 defined and hoping emacs will get a list to pick from,

I edit libvirt XML all the time.  The 'virsh edit' command is most
useful ...

I'd like to add my own rant about this though:

If an element isn't understood by libvirt, then libvirt just discards
it (without any indication or error, and without just remembering the
element in the XML).

This caused me a great deal of pain yesterday when I was adding a
watchdog/ element to a domain on an F12 machine, but the watchdog
didn't appear in the VM.  Later I discovered that libvirt on F12
predates the watchdog feature, and so it was just tossing away the
watchdog/ element completely from the XML ...

 which even if you did it right might just not work because the
 domain is running and your change would be discarded on
 restart... oops.

Which is also a bug.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into Xen guests.
http://et.redhat.com/~rjones/virt-p2v

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] My beefs with the libvirt XML configuration format

[Matthias Bolte]

2009/12/10 Diego Elio “Flameeyes” flamee...@gmail.com:
 Il giorno Thu, 10/12/2009 alle 18.28 +0100, Matthias Bolte ha scritto:

 The crash could be due to the QEMU monitor handling problem that was
 recently fixed.

 Yup that seems to be it, tied with a crash in qemu-kvm's 0.11.0 handling
 of virtio network (qemu crashed, which then caused libvirt to crash
 again) which is solved in 0.11.1.

 I'm sincerely not sure at this point if I should wait for 0.7.5 or look
 for a way to backport the needed change.


You can use the 0.7.4 release and apply this two commits to fix the crash:

http://libvirt.org/git/?p=libvirt.git;a=commit;h=79533da1b36cc16e0f3f8aea798d6cc20528c451
http://libvirt.org/git/?p=libvirt.git;a=commit;h=8e7d14953ca6ef047112bc6ac177507ea8f824f2

I tested it and it works for me.

Matthias

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] My beefs with the libvirt XML configuration format

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 05:49:39PM +, Richard W.M. Jones wrote:
 On Thu, Dec 10, 2009 at 05:44:43PM +0100, Daniel Veillard wrote:
Your viewpoint is that users should edit the XML. My viewpoint is that
  software should do this, basically in normal use of libvirt nobody
  should have to look at the XML, the virt-viewer/virt-install etc...
  tools should generate and handle those for you. It happens that one may
  have to tweak something like a pathname in a definition or something but
  whatever the level of schemas available it won't help for that kind of
  tweaks. Things like changing the ethernet type adapter should be a
  pull down list in a gui like virt-manager, not loading the saved xml in
  emacs, finding the associated relax-ng, finding the place where it's
  defined and hoping emacs will get a list to pick from,
 
 I edit libvirt XML all the time.  The 'virsh edit' command is most
 useful ...
 
 I'd like to add my own rant about this though:
 
 If an element isn't understood by libvirt, then libvirt just discards
 it (without any indication or error, and without just remembering the
 element in the XML).

There should be an option to validate the XML input, either by 
providing a VIR_DOMAIN_XML_VALIDATE flag with the APIs which 
accept XML as input, or by having virsh edit doing validation
after the editor exits. 

This would also allow virsh to re-launch the editor upon error
and let you correct the mistake instead of forcing you to start
again from scratch.

 This caused me a great deal of pain yesterday when I was adding a
 watchdog/ element to a domain on an F12 machine, but the watchdog
 didn't appear in the VM.  Later I discovered that libvirt on F12
 predates the watchdog feature, and so it was just tossing away the
 watchdog/ element completely from the XML ...

It is good that it throws away unknown elements. Having settings in
the XML that are used, but which might suddenly become active with
any upgrade is not good for ensuring a stable ABI for the guest.


  which even if you did it right might just not work because the
  domain is running and your change would be discarded on
  restart... oops.

That is not correct actually. It is possible with many of the drivers
(Xen, QEMU, LXC, UML) to change the persistent config, regardless of 
whether the domain is running.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH, TCK] Add missing module Test::Exception to the required module list

[Matthias Bolte]

---
 Build.PL |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/Build.PL b/Build.PL
index df35abf..453172c 100644
--- a/Build.PL
+++ b/Build.PL
@@ -69,6 +69,7 @@ my $b = $class-new(
'TAP::Formatter::HTML' = 0,
'TAP::Harness' = 3.11,
'TAP::Harness::Archive' = 0,
+   'Test::Exception' = 0,
'Test::Builder' = 0,
'Test::More' = 0,
'Sub::Uplevel' = 0,
-- 
1.6.0.4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH, TCK] Install tests to /usr/share/... instead of /share/...

[Matthias Bolte]

libvirt-tck expects the tests in /usr/share/libvirt-tck/tests.
---
 Build.PL |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/Build.PL b/Build.PL
index 453172c..c85a072 100644
--- a/Build.PL
+++ b/Build.PL
@@ -100,5 +100,5 @@ my $b = $class-new(
 $b-add_build_element(conf);
 $b-add_build_element(pkgdata);
 $b-install_path('conf' = File::Spec-catdir($b-install_base, etc, 
libvirt-tck));
-$b-install_path('pkgdata' = File::Spec-catdir($b-install_base, share, 
libvirt-tck, tests));
+$b-install_path('pkgdata' = File::Spec-catdir($b-install_base, 
usr/share, libvirt-tck, tests));
 $b-create_build_script;
-- 
1.6.0.4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH, TCK] Add missing module Test::Exception to the required module list

[Daniel P. Berrange]

On Thu, Dec 10, 2009 at 07:18:27PM +0100, Matthias Bolte wrote:
 ---
  Build.PL |1 +
  1 files changed, 1 insertions(+), 0 deletions(-)
 
 diff --git a/Build.PL b/Build.PL
 index df35abf..453172c 100644
 --- a/Build.PL
 +++ b/Build.PL
 @@ -69,6 +69,7 @@ my $b = $class-new(
   'TAP::Formatter::HTML' = 0,
   'TAP::Harness' = 3.11,
   'TAP::Harness::Archive' = 0,
 + 'Test::Exception' = 0,
   'Test::Builder' = 0,
   'Test::More' = 0,
   'Sub::Uplevel' = 0,

ACK

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] expose SR IOV physical/virtual function relationships

[Daniel P. Berrange]

On Fri, Dec 04, 2009 at 10:30:46AM -0500, Dave Allan wrote:
 Daniel P. Berrange wrote:
 
 I don't much like including the raw BDF format, because it is effectively
 adding a 3rd way of specifying PCI addresses in libvirt XML. We already
 have 2 different ways, which is 1 too many
 
 Node dev XML
 
   capability type='pci'
 domain0/domain
 bus0/bus
 slot31/slot
 function1/function
   /capability
 
 
 Domain XML for host devices  guest addresses
 
   address domain='0x' bus='0x00' slot='0x1f' function='0x5'/
 
 Yes, my bad for screwing up the nodedev  XML format when I designed
 it, stupidly choosing decimal instead of hex :-(
 
 I think we should make the virtual/physical function follow the domain
 XML style, with an address/ element. Perhaps also use nested capabilities
 in the way we did for NPIV host/vport stuff
 
   capability type='physical_function'
 address domain='0x' bus='0x00' slot='0x1f' function='0x1'/
   /capability
 
   capability type='virtual_functions'
 address domain='0x' bus='0x00' slot='0x1f' function='0x2'/
 address domain='0x' bus='0x00' slot='0x1f' function='0x3'/
 address domain='0x' bus='0x00' slot='0x1f' function='0x4'/
   /capability
 
 
 I'd even be inclined to retro-fit the existing capability type='pci'
 XML to duplicate the address info in this saner format eg
 
   capability type='pci'
 address domain='0x' bus='0x00' slot='0x1f' function='0x1' /
 domain0/domain
 bus0/bus
 slot31/slot
 function1/function
   /capability
 
 
 So, a PCI card with SR-IOV would end up looking like
 
   capability type='pci'
 domain0/domain
 bus0/bus
 slot31/slot
 function1/function
 address domain='0x' bus='0x00' slot='0x1f' function='0x1' /
 
 capability type='virtual_functions'
   address domain='0x' bus='0x00' slot='0x1f' function='0x2'/
   address domain='0x' bus='0x00' slot='0x1f' function='0x3'/
   address domain='0x' bus='0x00' slot='0x1f' function='0x4'/
 /capability
   /capability
 
 The nice thing about this kind of nesting is that it would let us show
 that a card is capable of exposing virtual functions, even if it does
 not currently have any enabled
 
 
 Regards,
 Daniel
 
 Here's an updated patch reflecting the above suggestions.
 
 Dave
 

 From d11d00d93c4bfbfd1125560ce80abfdbf88de94b Mon Sep 17 00:00:00 2001
 From: David Allan dal...@redhat.com
 Date: Mon, 30 Nov 2009 15:58:47 -0500
 Subject: [PATCH 1/1] Add SR IOV physical and virtual function relationships
 
 ---
  src/conf/node_device_conf.c   |   30 +
  src/conf/node_device_conf.h   |   16 +++
  src/node_device/node_device_driver.h  |   14 ++
  src/node_device/node_device_hal.c |6 +
  src/node_device/node_device_linux_sysfs.c |  200 
 -
  5 files changed, 265 insertions(+), 1 deletions(-)
 
 diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
 index 6003ab1..a8ecfb0 100644
 --- a/src/conf/node_device_conf.c
 +++ b/src/conf/node_device_conf.c
 @@ -246,6 +246,7 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
  {
  virBuffer buf = VIR_BUFFER_INITIALIZER;
  virNodeDevCapsDefPtr caps;
 +unsigned int i = 0;
  char *tmp;
 
  virBufferAddLit(buf, device\n);
 @@ -318,6 +319,30 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
data-pci_dev.vendor_name);
  else
  virBufferAddLit(buf,  /\n);
 +if (data-pci_dev.flags  
 VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION) {
 +virBufferAddLit(buf, capability type='physical 
 function'\n);

Can we change that to 'phys_function'

 +virBufferVSprintf(buf,
 +address domain='0x%.4x' 
 bus='0x%.2x' 
 +  slot='0x%.2x' function='0x%.1x'/\n,
 +  data-pci_dev.physical_function-domain,
 +  data-pci_dev.physical_function-bus,
 +  data-pci_dev.physical_function-slot,
 +  data-pci_dev.physical_function-function);
 +virBufferAddLit(buf, /capability\n);
 +}
 +if (data-pci_dev.flags  
 VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION) {
 +virBufferAddLit(buf, capability type='virtual 
 functions'\n);

And  'virt_function', because its nice to not have whitespace in the values

 +for (i = 0 ; i  data-pci_dev.num_virtual_functions ; i++) {
 +virBufferVSprintf(buf,
 +address domain='0x%.4x' 
 bus='0x%.2x' 
 +  slot='0x%.2x' function='0x%.1x'/\n,
 +  
 data-pci_dev.virtual_functions[i]-domain,
 +  
 

Re: [libvirt] [PATCH] expose SR IOV physical/virtual function relationships

[Dave Allan]

On 12/10/2009 03:16 PM, Daniel P. Berrange wrote:

On Fri, Dec 04, 2009 at 10:30:46AM -0500, Dave Allan wrote:

Daniel P. Berrange wrote:


I don't much like including the raw BDF format, because it is effectively
adding a 3rd way of specifying PCI addresses in libvirt XML. We already
have 2 different ways, which is 1 too many

Node dev XML

  capability type='pci'
domain0/domain
bus0/bus
slot31/slot
function1/function
  /capability


Domain XML for host devices  guest addresses

  address domain='0x' bus='0x00' slot='0x1f' function='0x5'/

Yes, my bad for screwing up the nodedev  XML format when I designed
it, stupidly choosing decimal instead of hex :-(

I think we should make the virtual/physical function follow the domain
XML style, with anaddress/  element. Perhaps also use nested capabilities
in the way we did for NPIV host/vport stuff

  capability type='physical_function'
address domain='0x' bus='0x00' slot='0x1f' function='0x1'/
  /capability

  capability type='virtual_functions'
address domain='0x' bus='0x00' slot='0x1f' function='0x2'/
address domain='0x' bus='0x00' slot='0x1f' function='0x3'/
address domain='0x' bus='0x00' slot='0x1f' function='0x4'/
  /capability


I'd even be inclined to retro-fit the existingcapability type='pci'
XML to duplicate the address info in this saner format eg

  capability type='pci'
address domain='0x' bus='0x00' slot='0x1f' function='0x1' /
domain0/domain
bus0/bus
slot31/slot
function1/function
  /capability


So, a PCI card with SR-IOV would end up looking like

  capability type='pci'
domain0/domain
bus0/bus
slot31/slot
function1/function
address domain='0x' bus='0x00' slot='0x1f' function='0x1' /

capability type='virtual_functions'
  address domain='0x' bus='0x00' slot='0x1f' function='0x2'/
  address domain='0x' bus='0x00' slot='0x1f' function='0x3'/
  address domain='0x' bus='0x00' slot='0x1f' function='0x4'/
/capability
  /capability

The nice thing about this kind of nesting is that it would let us show
that a card is capable of exposing virtual functions, even if it does
not currently have any enabled


Regards,
Daniel


Here's an updated patch reflecting the above suggestions.

Dave




 From d11d00d93c4bfbfd1125560ce80abfdbf88de94b Mon Sep 17 00:00:00 2001
From: David Allandal...@redhat.com
Date: Mon, 30 Nov 2009 15:58:47 -0500
Subject: [PATCH 1/1] Add SR IOV physical and virtual function relationships

---
  src/conf/node_device_conf.c   |   30 +
  src/conf/node_device_conf.h   |   16 +++
  src/node_device/node_device_driver.h  |   14 ++
  src/node_device/node_device_hal.c |6 +
  src/node_device/node_device_linux_sysfs.c |  200 -
  5 files changed, 265 insertions(+), 1 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 6003ab1..a8ecfb0 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -246,6 +246,7 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
  {
  virBuffer buf = VIR_BUFFER_INITIALIZER;
  virNodeDevCapsDefPtr caps;
+unsigned int i = 0;
  char *tmp;

  virBufferAddLit(buf, device\n);
@@ -318,6 +319,30 @@ char *virNodeDeviceDefFormat(virConnectPtr conn,
data-pci_dev.vendor_name);
  else
  virBufferAddLit(buf,  /\n);
+if (data-pci_dev.flags  
VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION) {
+virBufferAddLit(buf, capability type='physical 
function'\n);


Can we change that to 'phys_function'


+virBufferVSprintf(buf,
+  address domain='0x%.4x' bus='0x%.2x' 
+  slot='0x%.2x' function='0x%.1x'/\n,
+  data-pci_dev.physical_function-domain,
+  data-pci_dev.physical_function-bus,
+  data-pci_dev.physical_function-slot,
+  data-pci_dev.physical_function-function);
+virBufferAddLit(buf, /capability\n);
+}
+if (data-pci_dev.flags  
VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION) {
+virBufferAddLit(buf, capability type='virtual 
functions'\n);


And  'virt_function', because its nice to not have whitespace in the values


+for (i = 0 ; i  data-pci_dev.num_virtual_functions ; i++) {
+virBufferVSprintf(buf,
+  address domain='0x%.4x' bus='0x%.2x' 
+  slot='0x%.2x' function='0x%.1x'/\n,
+  
data-pci_dev.virtual_functions[i]-domain,
+  data-pci_dev.virtual_functions[i]-bus,
+  

[libvirt] [PATCH 01/12] Introduce a standardized data structure for device addresses

[Daniel P. Berrange]

All guest devices now use a common device address structure
summarized by:

  enum virDomainDeviceAddressType {
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE,
VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI,
  };

  enum virDomainDeviceAddressMode {
VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC,
VIR_DOMAIN_DEVICE_ADDRESS_MODE_STATIC,
  };

  struct _virDomainDevicePCIAddress {
unsigned int domain;
unsigned int bus;
unsigned int slot;
unsigned int function;
  };

  struct _virDomainDeviceAddress {
int type;
int mode;
union {
virDomainDevicePCIAddress pci;
} data;
  };

This replaces the anonymous structs in Disk/Net/Hostdev data
structures. Where available, the address is *always* printed
in the XML file, instead of being hidden in the internal state
file.

  address type='pci' mode='static' domain='0x' bus='0x1e' slot='0x07' 
function='0x0'/

A 'static' address is one assigned by the user, never changing,
while a 'dynamic' address is one assigned on the fly by QEMU.
'dynamic' addresses are thrown away when a VM stops.

The structure definition is based on Wolfgang Mauerer's disk
controller patch series.
---
 src/conf/domain_conf.c   |  432 ++---
 src/conf/domain_conf.h   |   90 ++-
 src/libvirt_private.syms |5 +
 src/qemu/qemu_driver.c   |   64 ---
 4 files changed, 418 insertions(+), 173 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index dca2e49..975b62b 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -88,6 +88,14 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
   hostdev,
   watchdog)
 
+VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
+  none,
+  pci)
+
+VIR_ENUM_IMPL(virDomainDeviceAddressMode, VIR_DOMAIN_DEVICE_ADDRESS_MODE_LAST,
+  dynamic,
+  static)
+
 VIR_ENUM_IMPL(virDomainDisk, VIR_DOMAIN_DISK_TYPE_LAST,
   block,
   file,
@@ -729,6 +737,245 @@ void virDomainRemoveInactive(virDomainObjListPtr doms,
 }
 
 
+int virDomainDeviceAddressIsValid(virDomainDeviceAddressPtr addr,
+  int type)
+{
+if (addr-type != type)
+return 0;
+
+switch (addr-type) {
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+return virDomainDevicePCIAddressIsValid(addr-data.pci);
+}
+
+return 0;
+}
+
+
+int virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr)
+{
+return addr-domain || addr-bus || addr-slot;
+}
+
+
+void virDomainDeviceAddressClear(virDomainDeviceAddressPtr addr)
+{
+memset(addr, 0, sizeof(addr));
+addr-type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE;
+}
+
+
+/* Generate a string representation of a device address
+ * @param address Device address to stringify
+ */
+static int virDomainDeviceAddressFormat(virBufferPtr buf,
+virDomainDeviceAddressPtr addr,
+int flags)
+{
+if (!addr) {
+virDomainReportError(NULL, VIR_ERR_INTERNAL_ERROR, %s,
+ _(missing address information));
+return -1;
+}
+
+if (addr-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+return 0;
+
+/* Don't output dynamically allocated addresses when doing inactive XML 
dump */
+if ((addr-mode == VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC) 
+(flags  VIR_DOMAIN_XML_INACTIVE))
+return 0;
+
+/* We'll be in domain/devices/[device type]/ so 3 level indent */
+virBufferVSprintf(buf,   address type='%s',
+  virDomainDeviceAddressTypeToString(addr-type));
+virBufferVSprintf(buf,  mode='%s',
+  virDomainDeviceAddressModeTypeToString(addr-mode));
+
+switch (addr-type) {
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+virBufferVSprintf(buf,  domain='0x%.4x' bus='0x%.2x' slot='0x%.2x' 
function='0x%.1x',
+  addr-data.pci.domain,
+  addr-data.pci.bus,
+  addr-data.pci.slot,
+  addr-data.pci.function);
+break;
+
+default:
+virDomainReportError(NULL, VIR_ERR_INTERNAL_ERROR,
+ _(unknown address type '%d'), addr-type);
+return -1;
+}
+
+virBufferAddLit(buf, /\n);
+
+return 0;
+}
+
+/* Compare two device addresses. Returns true if addresses are
+ * identical and false if the they differ (or are of different types)
+ * @param a, @para b Pointers to the addresses
+ */
+int virDomainDeviceAddressEqual(virDomainDeviceAddressPtr a,
+virDomainDeviceAddressPtr b)
+{
+if (!a || !b || a-type != b- type) {
+return 0;
+}
+
+switch (a-type) {
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
+return virDomainDevicePCIAddressEqual(a-data.pci,
+  

[libvirt] [PATCH 02/12] Extend the virDomainDeviceAddress struture to allow USB addresses

[Daniel P. Berrange]

Introduce a new structure

  struct _virDomainDeviceUSBAddress {
unsigned int bus;
unsigned int dev;
  };

and plug that into virDomainDeviceAddress. Convert the
host device USB config to use this new address struct.
XML looks like

address type='usb' bus='007' dev='003'/
---
 src/conf/domain_conf.c  |   93 --
 src/conf/domain_conf.h  |   15 +-
 src/libvirt_private.syms|2 +
 src/qemu/qemu_conf.c|   12 +++---
 src/qemu/qemu_driver.c  |   13 +++---
 src/security/security_selinux.c |   26 ++-
 6 files changed, 130 insertions(+), 31 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 975b62b..0e88362 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -90,7 +90,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
 
 VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
   none,
-  pci)
+  pci,
+  usb);
 
 VIR_ENUM_IMPL(virDomainDeviceAddressMode, VIR_DOMAIN_DEVICE_ADDRESS_MODE_LAST,
   dynamic,
@@ -746,6 +747,9 @@ int virDomainDeviceAddressIsValid(virDomainDeviceAddressPtr 
addr,
 switch (addr-type) {
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
 return virDomainDevicePCIAddressIsValid(addr-data.pci);
+
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+return virDomainDeviceUSBAddressIsValid(addr-data.usb);
 }
 
 return 0;
@@ -758,6 +762,12 @@ int 
virDomainDevicePCIAddressIsValid(virDomainDevicePCIAddressPtr addr)
 }
 
 
+int virDomainDeviceUSBAddressIsValid(virDomainDeviceUSBAddressPtr addr)
+{
+return addr-bus || addr-dev;
+}
+
+
 void virDomainDeviceAddressClear(virDomainDeviceAddressPtr addr)
 {
 memset(addr, 0, sizeof(addr));
@@ -801,6 +811,12 @@ static int virDomainDeviceAddressFormat(virBufferPtr buf,
   addr-data.pci.function);
 break;
 
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+virBufferVSprintf(buf,  bus='%.3d' dev='%.3d',
+  addr-data.usb.bus,
+  addr-data.usb.dev);
+break;
+
 default:
 virDomainReportError(NULL, VIR_ERR_INTERNAL_ERROR,
  _(unknown address type '%d'), addr-type);
@@ -827,6 +843,9 @@ int virDomainDeviceAddressEqual(virDomainDeviceAddressPtr a,
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI:
 return virDomainDevicePCIAddressEqual(a-data.pci,
   b-data.pci);
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+return virDomainDeviceUSBAddressEqual(a-data.usb,
+  b-data.usb);
 }
 
 return 0;
@@ -846,6 +865,17 @@ int 
virDomainDevicePCIAddressEqual(virDomainDevicePCIAddressPtr a,
 }
 
 
+int virDomainDeviceUSBAddressEqual(virDomainDeviceUSBAddressPtr a,
+   virDomainDeviceUSBAddressPtr b)
+{
+if (a-bus == b-bus 
+a-dev == b-dev)
+return 1;
+
+return 0;
+}
+
+
 static int
 virDomainDevicePCIAddressParseXML(virConnectPtr conn,
   xmlNodePtr node,
@@ -905,6 +935,47 @@ cleanup:
 return ret;
 }
 
+static int
+virDomainDeviceUSBAddressParseXML(virConnectPtr conn,
+  xmlNodePtr node,
+  virDomainDeviceUSBAddressPtr addr)
+{
+char *bus, *dev;
+int ret = -1;
+
+memset(addr, 0, sizeof(*addr));
+
+bus = virXMLPropString(node, bus);
+dev = virXMLPropString(node, dev);
+
+if (bus 
+virStrToLong_ui(bus, NULL, 10, addr-bus)  0) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot parse address 'bus' attribute));
+goto cleanup;
+}
+
+if (dev 
+virStrToLong_ui(dev, NULL, 10, addr-dev)  0) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot parse address 'dev' attribute));
+goto cleanup;
+}
+
+if (!virDomainDeviceUSBAddressIsValid(addr)) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(Insufficient specification for USB address));
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+VIR_FREE(bus);
+VIR_FREE(dev);
+return ret;
+}
+
 
 /* Parse the XML definition for a device address
  * @param node XML nodeset to parse for device address definition
@@ -960,6 +1031,11 @@ virDomainDeviceAddressParseXML(virConnectPtr conn,
 goto cleanup;
 break;
 
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
+if (virDomainDeviceUSBAddressParseXML(conn, node, addr-data.usb)  0)
+goto cleanup;
+break;
+
 default:
 /* Should not happen */
 virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
@@ -2518,7 +2594,7 @@ 

[libvirt] [PATCH 03/12] Extend the virDomainDeviceAddress struture to allow disk controller addresses

[Daniel P. Berrange]

Introduce a new structure

  struct _virDomainDeviceDriveAddress {
unsigned int controller;
unsigned int bus;
unsigned int unit;
  };

and plug that into virDomainDeviceAddress and generates XML that
looks like

  address type='drive' controller='1' bus='0' unit='5'/

* src/conf/domain_conf.h, src/conf/domain_conf.c: Parsing and
  formatting of drive addresses
---
 src/conf/domain_conf.c |   88 +++-
 src/conf/domain_conf.h |   13 +++
 2 files changed, 100 insertions(+), 1 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 0e88362..9c0b8cf 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -91,7 +91,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
 VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
   none,
   pci,
-  usb);
+  usb,
+  drive);
 
 VIR_ENUM_IMPL(virDomainDeviceAddressMode, VIR_DOMAIN_DEVICE_ADDRESS_MODE_LAST,
   dynamic,
@@ -750,6 +751,9 @@ int virDomainDeviceAddressIsValid(virDomainDeviceAddressPtr 
addr,
 
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
 return virDomainDeviceUSBAddressIsValid(addr-data.usb);
+
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+return virDomainDeviceDriveAddressIsValid(addr-data.drive);
 }
 
 return 0;
@@ -767,6 +771,11 @@ int 
virDomainDeviceUSBAddressIsValid(virDomainDeviceUSBAddressPtr addr)
 return addr-bus || addr-dev;
 }
 
+int virDomainDeviceDriveAddressIsValid(virDomainDeviceDriveAddressPtr addr 
ATTRIBUTE_UNUSED)
+{
+/*return addr-controller || addr-bus || addr-unit;*/
+return 1; /* 0 is valid for all fields, so any successfully parsed addr is 
valid */
+}
 
 void virDomainDeviceAddressClear(virDomainDeviceAddressPtr addr)
 {
@@ -817,6 +826,13 @@ static int virDomainDeviceAddressFormat(virBufferPtr buf,
   addr-data.usb.dev);
 break;
 
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+virBufferVSprintf(buf,  controller='%d' bus='%d' unit='%d',
+  addr-data.drive.controller,
+  addr-data.drive.bus,
+  addr-data.drive.unit);
+break;
+
 default:
 virDomainReportError(NULL, VIR_ERR_INTERNAL_ERROR,
  _(unknown address type '%d'), addr-type);
@@ -846,6 +862,9 @@ int virDomainDeviceAddressEqual(virDomainDeviceAddressPtr a,
 case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB:
 return virDomainDeviceUSBAddressEqual(a-data.usb,
   b-data.usb);
+case VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE:
+return virDomainDeviceDriveAddressEqual(a-data.drive,
+b-data.drive);
 }
 
 return 0;
@@ -876,6 +895,18 @@ int 
virDomainDeviceUSBAddressEqual(virDomainDeviceUSBAddressPtr a,
 }
 
 
+int virDomainDeviceDriveAddressEqual(virDomainDeviceDriveAddressPtr a,
+ virDomainDeviceDriveAddressPtr b)
+{
+if (a-controller == b-controller 
+a-bus == b-bus 
+a-unit == b-unit)
+return 1;
+
+return 0;
+}
+
+
 static int
 virDomainDevicePCIAddressParseXML(virConnectPtr conn,
   xmlNodePtr node,
@@ -977,6 +1008,56 @@ cleanup:
 }
 
 
+static int
+virDomainDeviceDriveAddressParseXML(virConnectPtr conn,
+xmlNodePtr node,
+virDomainDeviceDriveAddressPtr addr)
+{
+char *bus, *unit, *controller;
+int ret = -1;
+
+memset(addr, 0, sizeof(*addr));
+
+controller = virXMLPropString(node, controller);
+bus = virXMLPropString(node, bus);
+unit = virXMLPropString(node, unit);
+
+if (controller 
+virStrToLong_ui(controller, NULL, 10, addr-controller)  0) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot parse address 'controller' 
attribute));
+goto cleanup;
+}
+
+if (bus 
+virStrToLong_ui(bus, NULL, 10, addr-bus)  0) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot parse address 'bus' attribute));
+goto cleanup;
+}
+
+if (unit 
+virStrToLong_ui(unit, NULL, 10, addr-unit)  0) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(Cannot parse address 'unit' attribute));
+goto cleanup;
+}
+
+if (!virDomainDeviceDriveAddressIsValid(addr)) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(Insufficient specification for drive 
address));
+goto cleanup;
+}
+
+ret = 0;
+
+cleanup:
+VIR_FREE(controller);
+VIR_FREE(bus);
+VIR_FREE(unit);
+return ret;
+}
+
 /* Parse the XML definition for a 

[libvirt] [PATCH 04/12] Add address info to sound, video and watchdog devices

[Daniel P. Berrange]

Add the virDomainDeviceAddress information to the sound, video
and watchdog devices. This means all of them gain a new XML
element

  address  /

* src/conf/domain_conf.h: Add virDomainDeviceAddress to sound,
  video  watchdog device struts.
* src/conf/domain_conf.c: Hook up parsing/formatting for
  virDomainDeviceAddress in sound, video  watchdog devices
---
 src/conf/domain_conf.c |   85 ++-
 src/conf/domain_conf.h |3 ++
 2 files changed, 79 insertions(+), 9 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 9c0b8cf..a1eeb06 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2369,6 +2369,19 @@ virDomainSoundDefParseXML(virConnectPtr conn,
 
 char *model;
 virDomainSoundDefPtr def;
+xmlNodePtr addr = NULL;
+xmlNodePtr cur;
+
+cur = node-children;
+while (cur != NULL) {
+if (cur-type == XML_ELEMENT_NODE) {
+if ((addr == NULL) 
+xmlStrEqual(cur-name, BAD_CAST address)) {
+addr = cur;
+}
+}
+cur = cur-next;
+}
 
 if (VIR_ALLOC(def)  0) {
 virReportOOMError(conn);
@@ -2382,6 +2395,10 @@ virDomainSoundDefParseXML(virConnectPtr conn,
 goto error;
 }
 
+if (addr 
+virDomainDeviceAddressParseXML(conn, addr, def-addr, flags)  0)
+goto error;
+
 cleanup:
 VIR_FREE(model);
 
@@ -2397,11 +2414,24 @@ error:
 static virDomainWatchdogDefPtr
 virDomainWatchdogDefParseXML(virConnectPtr conn,
  const xmlNodePtr node,
- int flags ATTRIBUTE_UNUSED) {
+ int flags) {
 
 char *model = NULL;
 char *action = NULL;
 virDomainWatchdogDefPtr def;
+xmlNodePtr addr = NULL;
+xmlNodePtr cur;
+
+cur = node-children;
+while (cur != NULL) {
+if (cur-type == XML_ELEMENT_NODE) {
+if ((addr == NULL) 
+xmlStrEqual(cur-name, BAD_CAST address)) {
+addr = cur;
+}
+}
+cur = cur-next;
+}
 
 if (VIR_ALLOC (def)  0) {
 virReportOOMError (conn);
@@ -2433,6 +2463,10 @@ virDomainWatchdogDefParseXML(virConnectPtr conn,
 }
 }
 
+if (addr  (def-model == VIR_DOMAIN_WATCHDOG_MODEL_I6300ESB) 
+virDomainDeviceAddressParseXML(conn, addr, def-addr, flags)  0)
+goto error;
+
 cleanup:
 VIR_FREE (action);
 VIR_FREE (model);
@@ -2548,6 +2582,7 @@ virDomainVideoDefParseXML(virConnectPtr conn,
   int flags ATTRIBUTE_UNUSED) {
 virDomainVideoDefPtr def;
 xmlNodePtr cur;
+xmlNodePtr addr = NULL;
 char *type = NULL;
 char *heads = NULL;
 char *vram = NULL;
@@ -2566,6 +2601,9 @@ virDomainVideoDefParseXML(virConnectPtr conn,
 vram = virXMLPropString(cur, vram);
 heads = virXMLPropString(cur, heads);
 def-accel = virDomainVideoAccelDefParseXML(conn, cur);
+} else if ((addr == NULL) 
+   xmlStrEqual(cur-name, BAD_CAST address)) {
+addr = cur;
 }
 }
 cur = cur-next;
@@ -2605,6 +2643,10 @@ virDomainVideoDefParseXML(virConnectPtr conn,
 def-heads = 1;
 }
 
+if (addr 
+virDomainDeviceAddressParseXML(conn, addr, def-addr, flags)  0)
+goto error;
+
 VIR_FREE(type);
 VIR_FREE(vram);
 VIR_FREE(heads);
@@ -4645,7 +4687,8 @@ cleanup:
 static int
 virDomainSoundDefFormat(virConnectPtr conn,
 virBufferPtr buf,
-virDomainSoundDefPtr def)
+virDomainSoundDefPtr def,
+int flags)
 {
 const char *model = virDomainSoundModelTypeToString(def-model);
 
@@ -4655,9 +4698,18 @@ virDomainSoundDefFormat(virConnectPtr conn,
 return -1;
 }
 
-virBufferVSprintf(buf, sound model='%s'/\n,
+virBufferVSprintf(buf, sound model='%s',
   model);
 
+if (def-addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
+virBufferAddLit(buf, \n);
+if (virDomainDeviceAddressFormat(buf, def-addr, flags)  0)
+return -1;
+virBufferAddLit(buf, /sound\n);
+} else {
+virBufferAddLit(buf, /\n);
+}
+
 return 0;
 }
 
@@ -4665,7 +4717,8 @@ virDomainSoundDefFormat(virConnectPtr conn,
 static int
 virDomainWatchdogDefFormat(virConnectPtr conn,
virBufferPtr buf,
-   virDomainWatchdogDefPtr def)
+   virDomainWatchdogDefPtr def,
+   int flags)
 {
 const char *model = virDomainWatchdogModelTypeToString (def-model);
 const char *action = virDomainWatchdogActionTypeToString (def-action);
@@ -4682,9 +4735,18 @@ virDomainWatchdogDefFormat(virConnectPtr conn,
 return -1;
 }
 
-virBufferVSprintf(buf, 

[libvirt] [PATCH 05/12] Clear dynamic values from domain config at shutdown

[Daniel P. Berrange]

Some attributes in the guest config are only valid at runtime.
Although the XML parser/formatter are careful to avoid outputting
these attributes at the wrong time, there is still a risk that
the driver code may accidentally use them internally. We can
solve this by explicitly clearing all values

* src/conf/domain_conf.h, src/conf/domain_conf.c,
  src/libvirt_private.syms: Add virDomainDefClearDynamicValues
* src/lxc/lxc_driver.c, src/qemu/qemu_driver.c,
  src/uml/uml_driver.c: Call virDomainDefClearDynamicValues() when
  any guest shuts down.
---
 src/conf/domain_conf.c   |   36 
 src/conf/domain_conf.h   |2 ++
 src/libvirt_private.syms |2 ++
 src/lxc/lxc_driver.c |2 ++
 src/qemu/qemu_driver.c   |2 ++
 src/uml/uml_driver.c |2 ++
 6 files changed, 46 insertions(+), 0 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a1eeb06..533a1b0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -5286,6 +5286,42 @@ error:
 return NULL;
 }
 
+static void virDomainDeviceAddressClearDynamicValues(virDomainDeviceAddressPtr 
addr)
+{
+if (addr-mode == VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC)
+memset(addr, 0, sizeof(*addr));
+}
+
+void virDomainDefClearDynamicValues(virDomainDefPtr def)
+{
+int i;
+
+def-id = -1;
+
+for (i = 0; i  def-ngraphics ; i++) {
+if (def-graphics[i]-type == VIR_DOMAIN_GRAPHICS_TYPE_VNC 
+def-graphics[i]-data.vnc.autoport)
+def-graphics[i]-data.vnc.port = -1;
+else if (def-graphics[i]-type == VIR_DOMAIN_GRAPHICS_TYPE_RDP 
+ def-graphics[i]-data.rdp.autoport)
+def-graphics[i]-data.rdp.port = -1;
+}
+
+for (i = 0 ; i  def-ndisks ; i++) {
+virDomainDeviceAddressClearDynamicValues(def-disks[i]-addr);
+}
+for (i = 0 ; i  def-nhostdevs ; i++) {
+virDomainDeviceAddressClearDynamicValues(def-hostdevs[i]-addr);
+}
+for (i = 0 ; i  def-nnets ; i++) {
+virDomainDeviceAddressClearDynamicValues(def-nets[i]-addr);
+}
+
+if (def-seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC) {
+VIR_FREE(def-seclabel.label);
+VIR_FREE(def-seclabel.imagelabel);
+}
+}
 
 #ifndef PROXY
 
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index f06c2dc..c93aed6 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -775,6 +775,8 @@ char *virDomainObjFormat(virConnectPtr conn,
  virDomainObjPtr obj,
  int flags);
 
+void virDomainDefClearDynamicValues(virDomainDefPtr def);
+
 int virDomainCpuSetParse(virConnectPtr conn,
  const char **str,
  char sep,
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 49df15c..285715d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -161,6 +161,8 @@ virDomainDeviceAddressIsValid;
 virDomainDevicePCIAddressIsValid;
 virDomainDeviceUSBAddressIsValid;
 virDomainDeviceAddressClear;
+virDomainDeviceAddressTypeToString;
+virDomainDefClearDynamicValues;
 
 
 # domain_event.h
diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index c8e2dca..6a2fefb 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -749,6 +749,8 @@ static int lxcVmCleanup(virConnectPtr conn,
 vm-newDef = NULL;
 }
 
+virDomainDefClearDynamicValues(vm-def);
+
 return rc;
 }
 
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index da4fae7..85cbaf7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2617,6 +2617,8 @@ retry:
 vm-def-id = -1;
 vm-newDef = NULL;
 }
+
+virDomainDefClearDynamicValues(vm-def);
 }
 
 
diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index 48ef103..44d937f 100644
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -962,6 +962,8 @@ static void umlShutdownVMDaemon(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 vm-def-id = -1;
 vm-newDef = NULL;
 }
+
+virDomainDefClearDynamicValues(vm-def);
 }
 
 
-- 
1.6.5.2

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 06/12] Add new domain device: controller

[Daniel P. Berrange]

From: Wolfgang Mauerer wolfgang.maue...@siemens.com

This augments virDomainDevice with a controller element
that is used to represent disk controllers (e.g., scsi
controllers). The XML format is given by

  controller type=scsi index=num
 address type=pci domain=0xNUM bus=0xNUM slot=0xNUM/
  /controller

where type denotes the disk interface (scsi, ide,...), index
is an integer that identifies the controller for association
with disks, and the address element specifies the controller
address on the PCI bus as described in previous commits
The address element can be omitted; in this case, an address
will be assigned automatically.

Most of the code in this patch is from Wolfgang Mauerer's
previous disk controller series
---
 src/conf/domain_conf.c   |  207 +-
 src/conf/domain_conf.h   |   30 +++
 src/libvirt_private.syms |2 +
 3 files changed, 238 insertions(+), 1 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 533a1b0..95a9d52 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -86,7 +86,8 @@ VIR_ENUM_IMPL(virDomainDevice, VIR_DOMAIN_DEVICE_LAST,
   sound,
   video,
   hostdev,
-  watchdog)
+  watchdog,
+  controller)
 
 VIR_ENUM_IMPL(virDomainDeviceAddress, VIR_DOMAIN_DEVICE_ADDRESS_TYPE_LAST,
   none,
@@ -124,6 +125,12 @@ VIR_ENUM_IMPL(virDomainDiskCache, 
VIR_DOMAIN_DISK_CACHE_LAST,
   writethrough,
   writeback)
 
+VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST,
+  ide,
+  fdc,
+  scsi,
+  sata)
+
 VIR_ENUM_IMPL(virDomainFS, VIR_DOMAIN_FS_TYPE_LAST,
   mount,
   block,
@@ -371,6 +378,14 @@ void virDomainDiskDefFree(virDomainDiskDefPtr def)
 VIR_FREE(def);
 }
 
+void virDomainControllerDefFree(virDomainControllerDefPtr def)
+{
+if (!def)
+return;
+
+VIR_FREE(def);
+}
+
 void virDomainFSDefFree(virDomainFSDefPtr def)
 {
 if (!def)
@@ -523,6 +538,9 @@ void virDomainDeviceDefFree(virDomainDeviceDefPtr def)
 case VIR_DOMAIN_DEVICE_WATCHDOG:
 virDomainWatchdogDefFree(def-data.watchdog);
 break;
+case VIR_DOMAIN_DEVICE_CONTROLLER:
+virDomainControllerDefFree(def-data.controller);
+break;
 }
 
 VIR_FREE(def);
@@ -556,6 +574,10 @@ void virDomainDefFree(virDomainDefPtr def)
 virDomainDiskDefFree(def-disks[i]);
 VIR_FREE(def-disks);
 
+for (i = 0 ; i  def-ncontrollers ; i++)
+virDomainControllerDefFree(def-controllers[i]);
+VIR_FREE(def-controllers);
+
 for (i = 0 ; i  def-nfss ; i++)
 virDomainFSDefFree(def-fss[i]);
 VIR_FREE(def-fss);
@@ -1390,6 +1412,76 @@ cleanup:
 }
 
 
+/* Parse the XML definition for a controller
+ * @param node XML nodeset to parse for controller definition
+ */
+static virDomainControllerDefPtr
+virDomainControllerDefParseXML(virConnectPtr conn,
+  xmlNodePtr node,
+  int flags) {
+virDomainControllerDefPtr def;
+xmlNodePtr cur;
+char *type = NULL;
+char *idx = NULL;
+xmlNodePtr addr = NULL;
+
+if (VIR_ALLOC(def)  0) {
+virReportOOMError(conn);
+return NULL;
+}
+
+type = virXMLPropString(node, type);
+if (type) {
+if ((def-type = virDomainDiskBusTypeFromString(type))  0) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(unknown disk controller type '%s'), type);
+goto error;
+}
+}
+
+idx = virXMLPropString(node, index);
+if (idx) {
+if (virStrToLong_i(idx, NULL, 10, def-idx)  0) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot parse disk controller index %s), 
idx);
+goto error;
+}
+}
+
+cur = node-children;
+while (cur != NULL) {
+if (cur-type == XML_ELEMENT_NODE 
+addr == NULL) {
+addr = cur;
+}
+
+cur = cur-next;
+}
+
+if (addr) {
+if (virDomainDeviceAddressParseXML(conn, addr, def-addr,flags)  0)
+goto error;
+}
+
+if (def-addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE 
+def-addr.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, %s,
+ _(Disk controllers must use the 'pci' address 
type));
+goto error;
+}
+
+cleanup:
+VIR_FREE(type);
+VIR_FREE(idx);
+
+return def;
+
+ error:
+virDomainControllerDefFree(def);
+def = NULL;
+goto cleanup;
+}
+
 /* Parse the XML definition for a disk
  * @param node XML nodeset to parse for disk definition
  */
@@ -3092,6 +3184,10 @@ virDomainDeviceDefPtr 
virDomainDeviceDefParse(virConnectPtr conn,
 dev-type = 

[libvirt] [PATCH 08/12] Split code for building QEMU -drive arg in separate method

[Daniel P. Berrange]

To enable it to be called from multiple locations, split out
the code for building the -drive arg string

* src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Add qemuBuildDriveStr
  for building -drive arg string
---
 src/qemu/qemu_conf.c |  188 --
 src/qemu/qemu_conf.h |7 ++
 2 files changed, 112 insertions(+), 83 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 39d9314..27faa3f 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1313,6 +1313,110 @@ qemuAssignNetNames(virDomainDefPtr def,
 return 0;
 }
 
+#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \
+  abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_
+
+static int
+qemuSafeSerialParamValue(virConnectPtr conn,
+ const char *value)
+{
+if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen (value)) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(driver serial '%s' contains unsafe characters),
+ value);
+return -1;
+}
+
+return 0;
+}
+
+
+int
+qemuBuildDriveStr(virConnectPtr conn,
+  virDomainDiskDefPtr disk,
+  int bootable,
+  int qemuCmdFlags,
+  char **str)
+{
+virBuffer opt = VIR_BUFFER_INITIALIZER;
+const char *bus = virDomainDiskQEMUBusTypeToString(disk-bus);
+int idx = virDiskNameToIndex(disk-dst);
+char *optstr = NULL;
+
+if (idx  0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(unsupported disk type '%s'), disk-dst);
+goto error;
+}
+
+if (disk-src) {
+if (disk-type == VIR_DOMAIN_DISK_TYPE_DIR) {
+/* QEMU only supports magic FAT format for now */
+if (disk-driverType 
+STRNEQ(disk-driverType, fat)) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(unsupported disk driver type for '%s'),
+ disk-driverType);
+goto error;
+}
+if (!disk-readonly) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, 
%s,
+ _(cannot create virtual FAT disks in 
read-write mode));
+goto error;
+}
+if (disk-device == VIR_DOMAIN_DISK_DEVICE_FLOPPY)
+virBufferVSprintf(opt, file=fat:floppy:%s,, disk-src);
+else
+virBufferVSprintf(opt, file=fat:%s,, disk-src);
+} else {
+virBufferVSprintf(opt, file=%s,, disk-src);
+}
+}
+virBufferVSprintf(opt, if=%s, bus);
+if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM)
+virBufferAddLit(opt, ,media=cdrom);
+virBufferVSprintf(opt, ,index=%d, idx);
+if (bootable 
+disk-device == VIR_DOMAIN_DISK_DEVICE_DISK)
+virBufferAddLit(opt, ,boot=on);
+if (disk-driverType 
+disk-type != VIR_DOMAIN_DISK_TYPE_DIR 
+qemuCmdFlags  QEMUD_CMD_FLAG_DRIVE_FORMAT)
+virBufferVSprintf(opt, ,format=%s, disk-driverType);
+if (disk-serial 
+(qemuCmdFlags  QEMUD_CMD_FLAG_DRIVE_SERIAL)) {
+if (qemuSafeSerialParamValue(conn, disk-serial)  0)
+goto error;
+virBufferVSprintf(opt, ,serial=%s, disk-serial);
+}
+
+if (disk-cachemode) {
+const char *mode =
+(qemuCmdFlags  QEMUD_CMD_FLAG_DRIVE_CACHE_V2) ?
+qemuDiskCacheV2TypeToString(disk-cachemode) :
+qemuDiskCacheV1TypeToString(disk-cachemode);
+
+virBufferVSprintf(opt, ,cache=%s, mode);
+} else if (disk-shared  !disk-readonly) {
+virBufferAddLit(opt, ,cache=off);
+}
+
+if (virBufferError(opt)) {
+virReportOOMError(conn);
+goto error;
+}
+
+*str = virBufferContentAndReset(opt);
+return 0;
+
+error:
+optstr = virBufferContentAndReset(opt);
+VIR_FREE(optstr);
+*str = NULL;
+return -1;
+}
+
+
 int
 qemuBuildNicStr(virConnectPtr conn,
 virDomainNetDefPtr net,
@@ -1562,23 +1666,6 @@ static void 
qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev,
 }
 }
 
-#define QEMU_SERIAL_PARAM_ACCEPTED_CHARS \
-  abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_
-
-static int
-qemuSafeSerialParamValue(virConnectPtr conn,
- const char *value)
-{
-if (strspn(value, QEMU_SERIAL_PARAM_ACCEPTED_CHARS) != strlen (value)) {
-qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _(driver serial '%s' contains unsafe characters),
- value);
-return -1;
-}
-
-return 0;
-}
-
 /*
  * Constructs a argv suitable for launching qemu with config defined
  * for a given virtual machine.
@@ -1960,12 +2047,9 @@ int qemudBuildCommandLine(virConnectPtr conn,
 }
 
  

[libvirt] [PATCH 09/12] Specify bus/unit instead of index for disks with QEMU

[Daniel P. Berrange]

The current code for using -drive simply sets the -drive 'index'
parameter. QEMU internally converts this to bus/unit depending
on the type of drive. This does not give us precise control over
the bus/unit assignment though. This change switches over to make
libvirt explicitly calculate the bus/unit number.

In addition bus/unit/index are actually irrelevant for VirtIO
disks, since each virtio disk is a separate PCI device. No disk
controller is involved.

Doing the conversion to bus/unit in libvirt allows us to correctly
attach SCSI controllers when required.

* src/qemu/qemu_conf.c: Specify bus/unit instead of index for
  disks
---
 src/qemu/qemu_conf.c |   80 +-
 1 files changed, 79 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index 27faa3f..f4fbe96 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1342,6 +1342,7 @@ qemuBuildDriveStr(virConnectPtr conn,
 const char *bus = virDomainDiskQEMUBusTypeToString(disk-bus);
 int idx = virDiskNameToIndex(disk-dst);
 char *optstr = NULL;
+int controllerid = -1, busid = -1, unitid = -1;
 
 if (idx  0) {
 qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -1349,6 +1350,80 @@ qemuBuildDriveStr(virConnectPtr conn,
 goto error;
 }
 
+switch (disk-bus) {
+case VIR_DOMAIN_DISK_BUS_SCSI:
+case VIR_DOMAIN_DISK_BUS_IDE:
+case VIR_DOMAIN_DISK_BUS_FDC:
+if (disk-addr.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE 
+disk-addr.mode == VIR_DOMAIN_DEVICE_ADDRESS_MODE_STATIC) {
+/* The user specified an explicit address, so ignore dev name */
+controllerid = disk-addr.data.drive.controller;
+busid = disk-addr.data.drive.bus;
+unitid = disk-addr.data.drive.unit;
+} else if (disk-addr.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE ||
+   disk-addr.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE) {
+/*
+ * We auto-assign contoller/bus/unit based on the drive
+ * index, which was in turn based on dev name eg sda
+ */
+if (disk-bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+/* Many SCSI controllers, only 1 bus, each with 7 units */
+controllerid = idx / 7;
+busid = 0;
+unitid = idx % 7;
+} else if (disk-bus == VIR_DOMAIN_DISK_BUS_IDE) {
+/* One IDE controller, with 2 buses, each 2 units */
+controllerid = 0;
+busid = idx / 2;
+unitid = idx % 2;
+} else {
+controllerid = 0;
+busid = 0;
+unitid = idx;
+}
+
+disk-addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_DRIVE;
+disk-addr.mode = VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC;
+disk-addr.data.drive.controller = controllerid;
+disk-addr.data.drive.bus = busid;
+disk-addr.data.drive.unit = unitid;
+} else {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, %s,
+ _(unexpected address type for ide/scsi/fdc 
disk));
+goto error;
+}
+break;
+
+case VIR_DOMAIN_DISK_BUS_VIRTIO:
+/* Each virtio drive is a separate PCI device, no unit/busid */
+break;
+
+case VIR_DOMAIN_DISK_BUS_XEN:
+/* Xen has no address type currently, so assign based on index */
+unitid = idx;
+break;
+}
+
+if (disk-bus == VIR_DOMAIN_DISK_BUS_SCSI) {
+if (busid != 0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(SCSI controller only supports 1 bus));
+goto error;
+}
+/* Setting bus= attr for SCSI drives, causes a controller
+ * to be created. Yes this is slightly odd. It is not possible
+ * to have  1 bus on a SCSI controller (yet). */
+busid = controllerid;
+} else if (disk-bus == VIR_DOMAIN_DISK_BUS_IDE ||
+   disk-bus == VIR_DOMAIN_DISK_BUS_FDC) {
+/* We can only have 1 IDE / Floppy controller (currently) */
+if (controllerid != 0) {
+qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(Only 1 %s controller is supported), bus);
+goto error;
+}
+}
+
 if (disk-src) {
 if (disk-type == VIR_DOMAIN_DISK_TYPE_DIR) {
 /* QEMU only supports magic FAT format for now */
@@ -1375,7 +1450,10 @@ qemuBuildDriveStr(virConnectPtr conn,
 virBufferVSprintf(opt, if=%s, bus);
 if (disk-device == VIR_DOMAIN_DISK_DEVICE_CDROM)
 virBufferAddLit(opt, ,media=cdrom);
-virBufferVSprintf(opt, ,index=%d, idx);
+if (busid != -1)
+virBufferVSprintf(opt, ,bus=%d, busid);
+if (unitid != -1)
+virBufferVSprintf(opt, 

[libvirt] [PATCH 10/12] Implement SCSI controller hotplug/unplug for QEMU

[Daniel P. Berrange]

From: Wolfgang Mauerer wolfgang.maue...@siemens.com

This patch allows for explicit hotplug/unplug of SCSI controllers.
Ordinarily this is not required, since QEMU/libvirt will attach
a new SCSI controller whenever one is required. Allowing explicit
hotplug of controllers though, enables the caller to specify a
static PCI address, instead of auto-assigning the next available
PCI slot. Or it will when we have static PCI addressing.

This patch is derived from Wolfgang Mauerer's disk controller
patch series.

* src/qemu/qemu_driver.c: Support hotplug  unplug of SCSI
  controllers
* src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h,
  src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h,
  src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add
  new API for attaching PCI SCSI controllers
---
 src/libvirt_private.syms |2 +
 src/qemu/qemu_driver.c   |  117 ++
 src/qemu/qemu_monitor.c  |   16 ++
 src/qemu/qemu_monitor.h  |3 +
 src/qemu/qemu_monitor_json.c |   40 ++
 src/qemu/qemu_monitor_json.h |4 ++
 src/qemu/qemu_monitor_text.c |   43 +++
 src/qemu/qemu_monitor_text.h |5 ++
 8 files changed, 230 insertions(+), 0 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 8622e5b..3e98800 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -165,6 +165,8 @@ virDomainDeviceUSBAddressIsValid;
 virDomainDeviceAddressClear;
 virDomainDeviceAddressTypeToString;
 virDomainDefClearDynamicValues;
+virDomainControllerTypeToString;
+virDomainControllerDefFree;
 
 
 # domain_event.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 321cd4e..0a91c2a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4961,6 +4961,44 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr 
conn,
 return ret;
 }
 
+static int qemudDomainAttachPciControllerDevice(virConnectPtr conn,
+struct qemud_driver *driver,
+virDomainObjPtr vm,
+virDomainDeviceDefPtr dev)
+{
+int i, ret;
+const char* type = 
virDomainControllerTypeToString(dev-data.controller-type);
+qemuDomainObjPrivatePtr priv = vm-privateData;
+
+for (i = 0 ; i  vm-def-ncontrollers ; i++) {
+if ((vm-def-controllers[i]-type == dev-data.controller-type) 
+(vm-def-controllers[i]-idx == dev-data.controller-idx)) {
+qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
+ _(target %s:%d already exists),
+ type, dev-data.controller-idx);
+return -1;
+}
+}
+
+if (VIR_REALLOC_N(vm-def-controllers, vm-def-ncontrollers+1)  0) {
+virReportOOMError(conn);
+return -1;
+}
+
+qemuDomainObjEnterMonitorWithDriver(driver, vm);
+ret = qemuMonitorAttachPCIDiskController(priv-mon,
+ type,
+ 
dev-data.controller-addr.data.pci);
+qemuDomainObjExitMonitorWithDriver(driver, vm);
+
+if (ret == 0) {
+dev-data.controller-addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+virDomainControllerInsertPreAlloced(vm-def, dev-data.controller);
+}
+
+return ret;
+}
+
 static int qemudDomainAttachUsbMassstorageDevice(virConnectPtr conn,
  struct qemud_driver *driver,
  virDomainObjPtr vm,
@@ -5345,6 +5383,15 @@ static int qemudDomainAttachDevice(virDomainPtr dom,
 virCgroupDenyDevicePath(cgroup,
 dev-data.disk-src);
 }
+} else if (dev-type == VIR_DOMAIN_DEVICE_CONTROLLER) {
+if (dev-data.controller-type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) {
+ret = qemudDomainAttachPciControllerDevice(dom-conn, driver, vm, 
dev);
+} else {
+qemudReportError(dom-conn, dom, NULL, VIR_ERR_NO_SUPPORT,
+ _(disk controller bus '%s' cannot be 
hotplugged.),
+ 
virDomainControllerTypeToString(dev-data.controller-type));
+/* fallthrough */
+}
 } else if (dev-type == VIR_DOMAIN_DEVICE_NET) {
 ret = qemudDomainAttachNetDevice(dom-conn, driver, vm, dev, 
qemuCmdFlags);
 } else if (dev-type == VIR_DOMAIN_DEVICE_HOSTDEV) {
@@ -5436,6 +5483,67 @@ cleanup:
 return ret;
 }
 
+static int qemudDomainDetachPciControllerDevice(virConnectPtr conn,
+struct qemud_driver *driver,
+virDomainObjPtr vm,
+virDomainDeviceDefPtr dev)
+{
+int i, ret = -1;
+virDomainControllerDefPtr detach = NULL;
+

[libvirt] [PATCH 12/12] Detect PCI addresses at QEMU startup

[Daniel P. Berrange]

Hotunplug of devices requires that we know their PCI address. Even
hotplug of SCSI drives, required that we know the PCI address of
the SCSI controller to attach the drive to. We can find this out
by running 'info pci' and then correlating the vendor/product IDs
with the devices we booted with.

* src/qemu/qemu_driver.c: Assign all dynamic PCI addresses on
  startup of QEMU VM, matching vendor/product IDs
* src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h,
  src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h,
  src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add
  API for fetching PCI device address mapping
---
 src/qemu/qemu_driver.c   |  365 ++
 src/qemu/qemu_monitor.c  |   13 ++
 src/qemu/qemu_monitor.h  |   11 ++
 src/qemu/qemu_monitor_json.c |7 +
 src/qemu/qemu_monitor_json.h |3 +
 src/qemu/qemu_monitor_text.c |  129 +++
 src/qemu/qemu_monitor_text.h |2 +
 7 files changed, 530 insertions(+), 0 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 527b5d7..999062d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -1598,6 +1598,368 @@ qemuInitPasswords(struct qemud_driver *driver,
 }
 
 
+#define QEMU_PCI_VENDOR_INTEL 0x8086
+#define QEMU_PCI_VENDOR_LSI_LOGIC 0x1000
+#define QEMU_PCI_VENDOR_REDHAT0x1af4
+#define QEMU_PCI_VENDOR_CIRRUS0x1013
+#define QEMU_PCI_VENDOR_REALTEK   0x10ec
+#define QEMU_PCI_VENDOR_AMD   0x1022
+#define QEMU_PCI_VENDOR_ENSONIQ   0x1274
+#define QEMU_PCI_VENDOR_VMWARE0x15ad
+#define QEMU_PCI_VENDOR_QEMU  0x1234
+
+#define QEMU_PCI_PRODUCT_DISK_VIRTIO 0x1001
+
+#define QEMU_PCI_PRODUCT_NIC_NE2K 0x8029
+#define QEMU_PCI_PRODUCT_NIC_PCNET0x2000
+#define QEMU_PCI_PRODUCT_NIC_RTL8139  0x8139
+#define QEMU_PCI_PRODUCT_NIC_E10000x100E
+#define QEMU_PCI_PRODUCT_NIC_VIRTIO   0x1000
+
+#define QEMU_PCI_PRODUCT_VGA_CIRRUS 0x00b8
+#define QEMU_PCI_PRODUCT_VGA_VMWARE 0x0405
+#define QEMU_PCI_PRODUCT_VGA_STDVGA 0x
+
+#define QEMU_PCI_PRODUCT_AUDIO_AC970x2415
+#define QEMU_PCI_PRODUCT_AUDIO_ES1370  0x5000
+
+#define QEMU_PCI_PRODUCT_CONTROLLER_PIIX 0x7010
+#define QEMU_PCI_PRODUCT_CONTROLLER_LSI  0x0012
+
+#define QEMU_PCI_PRODUCT_WATCHDOG_I63000ESB 0x25ab
+
+static int
+qemuAssignNextPCIAddress(virDomainDeviceAddress *addr,
+ int vendor,
+ int product,
+ qemuMonitorPCIAddress *addrs,
+ int naddrs)
+{
+int found = 0;
+int i;
+
+VIR_DEBUG(Look for %x:%x out of %d, vendor, product, naddrs);
+
+for (i = 0 ; (i  naddrs)  !found; i++) {
+VIR_DEBUG(Maybe %x:%x, addrs[i].vendor, addrs[i].product);
+if (addrs[i].vendor == vendor 
+addrs[i].product == product) {
+VIR_DEBUG(Match %d, i);
+found = 1;
+break;
+}
+}
+if (!found) {
+return -1;
+}
+
+/* Blank it out so this device isn't matched again */
+addrs[i].vendor = 0;
+addrs[i].product = 0;
+
+if (addr-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE)
+addr-type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+
+if (addr-type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
+if (addr-mode == VIR_DOMAIN_DEVICE_ADDRESS_MODE_DYNAMIC) {
+addr-data.pci.domain = addrs[i].addr.domain;
+addr-data.pci.bus = addrs[i].addr.bus;
+addr-data.pci.slot = addrs[i].addr.slot;
+addr-data.pci.function = addrs[i].addr.function;
+} else {
+/* XXX should we validate the device addr matches
+ * the static addr we requested ? */
+}
+}
+
+return 0;
+}
+
+static int
+qemuGetPCIDiskVendorProduct(virDomainDiskDefPtr def,
+unsigned *vendor,
+unsigned *product)
+{
+switch (def-bus) {
+case VIR_DOMAIN_DISK_BUS_VIRTIO:
+*vendor = QEMU_PCI_VENDOR_REDHAT;
+*product = QEMU_PCI_PRODUCT_DISK_VIRTIO;
+break;
+
+default:
+return -1;
+}
+
+return 0;
+}
+
+static int
+qemuGetPCINetVendorProduct(virDomainNetDefPtr def,
+unsigned *vendor,
+unsigned *product)
+{
+if (!def-model)
+return -1;
+
+if (STREQ(def-model, ne2k_pci)) {
+*vendor = QEMU_PCI_VENDOR_REALTEK;
+*product = QEMU_PCI_PRODUCT_NIC_NE2K;
+} else if (STREQ(def-model, pcnet)) {
+*vendor = QEMU_PCI_VENDOR_AMD;
+*product = QEMU_PCI_PRODUCT_NIC_PCNET;
+} else if (STREQ(def-model, rtl8139)) {
+*vendor = QEMU_PCI_VENDOR_REALTEK;
+*product = QEMU_PCI_PRODUCT_NIC_RTL8139;
+} else if (STREQ(def-model, e1000)) {
+*vendor = QEMU_PCI_VENDOR_INTEL;
+*product = QEMU_PCI_PRODUCT_NIC_E1000;
+} else if (STREQ(def-model, virtio)) {
+*vendor = QEMU_PCI_VENDOR_REDHAT;
+*product = 

[libvirt] [PATCH 11/12] Properly support SCSI drive hotplug

[Daniel P. Berrange]

The current SCSI hotplug support attaches a brand new SCSI controller
for every disk. This is broken because the semantics differ from those
used when starting the VM initially. In the latter case, each SCSI
controller is filled before a new one is added.

If the user specifies an high drive index (sdazz) then at initial
startup, many intermediate SCSI controllers may be added with no
drives.

This patch changes SCSI hotplug so that it exactly matches the
behaviour of initial startup. First the SCSI controller number is
determined for the drive to be hotplugged. If any controller upto
and including that controller number is not yet present, it is
attached. Then finally the drive is attached to the last controller.

NB, this breaks SCSI hotunplug, because there is no 'drive_del'
command in current QEMU. Previous SCSI hotunplug was broken in
any case because it was unplugging the entire controller, not
just the drive in question.

A future QEMU will allow proper SCSI hotunplug of a drive.

This patch is derived from work done by Wolfgang Mauerer on disk
controllers.

* src/qemu/qemu_driver.c: Fix SCSI hotplug to add a drive to
 the correct controller, instead of just attaching a new
  controller.
* src/qemu/qemu_monitor.c, src/qemu/qemu_monitor.h,
  src/qemu/qemu_monitor_json.c, src/qemu/qemu_monitor_json.h,
  src/qemu/qemu_monitor_text.c, src/qemu/qemu_monitor_text.h: Add
  support for 'drive_add' command
---
 src/libvirt_private.syms |1 +
 src/qemu/qemu_driver.c   |  145 +
 src/qemu/qemu_monitor.c  |   20 ++
 src/qemu/qemu_monitor.h  |5 ++
 src/qemu/qemu_monitor_json.c |   81 +--
 src/qemu/qemu_monitor_json.h |5 ++
 src/qemu/qemu_monitor_text.c |  102 +
 src/qemu/qemu_monitor_text.h |5 ++
 8 files changed, 342 insertions(+), 22 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 3e98800..963f9b2 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -167,6 +167,7 @@ virDomainDeviceAddressTypeToString;
 virDomainDefClearDynamicValues;
 virDomainControllerTypeToString;
 virDomainControllerDefFree;
+virDomainDeviceAddressTypeToString;
 
 
 # domain_event.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0a91c2a..527b5d7 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4964,18 +4964,18 @@ static int qemudDomainAttachPciDiskDevice(virConnectPtr 
conn,
 static int qemudDomainAttachPciControllerDevice(virConnectPtr conn,
 struct qemud_driver *driver,
 virDomainObjPtr vm,
-virDomainDeviceDefPtr dev)
+virDomainControllerDefPtr dev)
 {
 int i, ret;
-const char* type = 
virDomainControllerTypeToString(dev-data.controller-type);
+const char* type = virDomainControllerTypeToString(dev-type);
 qemuDomainObjPrivatePtr priv = vm-privateData;
 
 for (i = 0 ; i  vm-def-ncontrollers ; i++) {
-if ((vm-def-controllers[i]-type == dev-data.controller-type) 
-(vm-def-controllers[i]-idx == dev-data.controller-idx)) {
+if ((vm-def-controllers[i]-type == dev-type) 
+(vm-def-controllers[i]-idx == dev-idx)) {
 qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
  _(target %s:%d already exists),
- type, dev-data.controller-idx);
+ type, dev-idx);
 return -1;
 }
 }
@@ -4988,17 +4988,130 @@ static int 
qemudDomainAttachPciControllerDevice(virConnectPtr conn,
 qemuDomainObjEnterMonitorWithDriver(driver, vm);
 ret = qemuMonitorAttachPCIDiskController(priv-mon,
  type,
- 
dev-data.controller-addr.data.pci);
+ dev-addr.data.pci);
 qemuDomainObjExitMonitorWithDriver(driver, vm);
 
 if (ret == 0) {
-dev-data.controller-addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
-virDomainControllerInsertPreAlloced(vm-def, dev-data.controller);
+dev-addr.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI;
+virDomainControllerInsertPreAlloced(vm-def, dev);
 }
 
 return ret;
 }
 
+static virDomainControllerDefPtr
+qemuDomainFindOrCreateSCSIDiskController(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainObjPtr vm,
+ int controller)
+{
+int i;
+virDomainControllerDefPtr cont;
+for (i = 0 ; i  vm-def-ncontrollers ; i++) {
+cont = vm-def-controllers[i];
+
+if (cont-type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
+continue;
+
+if 

[libvirt] [PATCH] Fix memory leak in virStorageBackendCopyToFD

[Matthias Bolte]

---
 src/storage/storage_backend.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index a3b4d5a..9dc801c 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -198,6 +198,8 @@ cleanup:
 if (inputfd != -1)
 close(inputfd);
 
+VIR_FREE(buf);
+
 return ret;
 }
 
-- 
1.6.0.4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Fix reference leak in remoteDispatchStorageVolCreateXmlFrom

[Matthias Bolte]

---
 daemon/remote.c |3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/daemon/remote.c b/daemon/remote.c
index 3117615..7a43046 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -4118,12 +4118,15 @@ remoteDispatchStorageVolCreateXmlFrom (struct 
qemud_server *server ATTRIBUTE_UNU
 
 clonevol = get_nonnull_storage_vol (conn, args-clonevol);
 if (clonevol == NULL) {
+virStoragePoolFree(pool);
 remoteDispatchConnError(rerr, conn);
 return -1;
 }
 
 newvol = virStorageVolCreateXMLFrom (pool, args-xml, clonevol,
  args-flags);
+virStorageVolFree(clonevol);
+virStoragePoolFree(pool);
 if (newvol == NULL) {
 remoteDispatchConnError(rerr, conn);
 return -1;
-- 
1.6.0.4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH] Remove iptables.c from POTFILES.in

[Matthias Bolte]

---
 po/POTFILES.in |1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/po/POTFILES.in b/po/POTFILES.in
index f8e9130..a429b19 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -50,7 +50,6 @@ src/uml/uml_conf.c
 src/uml/uml_driver.c
 src/util/bridge.c
 src/util/conf.c
-src/util/iptables.c
 src/util/json.c
 src/util/logging.c
 src/util/pci.c
-- 
1.6.0.4

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] My beefs with the libvirt XML configuration format

[Daniel Veillard]

On Thu, Dec 10, 2009 at 06:07:13PM +, Daniel P. Berrange wrote:
 On Thu, Dec 10, 2009 at 05:49:39PM +, Richard W.M. Jones wrote:
  On Thu, Dec 10, 2009 at 05:44:43PM +0100, Daniel Veillard wrote:
 Your viewpoint is that users should edit the XML. My viewpoint is that
   software should do this, basically in normal use of libvirt nobody
   should have to look at the XML, the virt-viewer/virt-install etc...
   tools should generate and handle those for you. It happens that one may
   have to tweak something like a pathname in a definition or something but
   whatever the level of schemas available it won't help for that kind of
   tweaks. Things like changing the ethernet type adapter should be a
   pull down list in a gui like virt-manager, not loading the saved xml in
   emacs, finding the associated relax-ng, finding the place where it's
   defined and hoping emacs will get a list to pick from,
  
  I edit libvirt XML all the time.  The 'virsh edit' command is most
  useful ...
  
  I'd like to add my own rant about this though:
  
  If an element isn't understood by libvirt, then libvirt just discards
  it (without any indication or error, and without just remembering the
  element in the XML).

  Remembering is a hard feature and probably not worth it in libvirt
case (but I'm sure I argued with dan over this at the very beginning :-)
it makes sense for editor like applications or when you know you need
to mix vocabularies, for libvirt, I think it's overkill and would
make the code significantly more complex.

 There should be an option to validate the XML input, either by 
 providing a VIR_DOMAIN_XML_VALIDATE flag with the APIs which 
 accept XML as input, or by having virsh edit doing validation
 after the editor exits. 

  I think I suggested a couple of time to have the input XML data
be validated at the API level, but we don't want to do this
systematically, this would create IMHO more problems it can solve.
Using a flag and/or activating it when libvirt conf is in debug mode
would both make sense.

 This would also allow virsh to re-launch the editor upon error
 and let you correct the mistake instead of forcing you to start
 again from scratch.

  The schemas validation won't be perfect in any way, for example
trying to limit the list of allowed ethernet adapter based on the
hypervisor type is nearly impossible even with Relax-NG since we
differentiate based on an attribute in the top level element (this
would force to basically write parallel schemas and become completely
unmaintainable). Relax-NG validation also will provide out of context
error messages, while the conf parser can give way better diagnostics.

  This caused me a great deal of pain yesterday when I was adding a
  watchdog/ element to a domain on an F12 machine, but the watchdog
  didn't appear in the VM.  Later I discovered that libvirt on F12
  predates the watchdog feature, and so it was just tossing away the
  watchdog/ element completely from the XML ...
 
 It is good that it throws away unknown elements. Having settings in
 the XML that are used, but which might suddenly become active with
 any upgrade is not good for ensuring a stable ABI for the guest.
 
 
   which even if you did it right might just not work because the
   domain is running and your change would be discarded on
   restart... oops.
 
 That is not correct actually. It is possible with many of the drivers
 (Xen, QEMU, LXC, UML) to change the persistent config, regardless of 
 whether the domain is running.

  I used that as an example, the point being that decoupling the config
modification process from the actual application using the definition
makes some mistakes uncheckable either at the system level or even at
teh syntactic level, to take again the example of changing the ethernet
adapter while a specific tool like virt-manager may have the logic
needed to be able to provide a list based on the hypervisor type,
the Relax-NG will never embbed that logic.

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] Replace optionaloneOrMore with zeroOrMore.

[Daniel Veillard]

On Thu, Dec 10, 2009 at 05:18:15PM +0100, Diego Elio 'Flameeyes' Pettenò wrote:
 The two syntax should be equivalent, but the former creates, with trang, an
 invalid Relax-NG Compact form file, as the output will contain the sequence
 +? which is not recognized by tools like rnv.
 ---
  docs/schemas/domain.rng |   40 +++-
  1 files changed, 19 insertions(+), 21 deletions(-)

  Well, that's really a trang bug and should be reported, but the
workaround is fine, applied, thanks !

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
dan...@veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list