date:20090126

On Thu, Jan 22, 2009 at 07:13:47PM +, Daniel P. Berrange wrote:
 On Thu, Jan 22, 2009 at 08:09:51PM +0100, Jim Meyering wrote:
  Merge error?
  I'll commit this in an hour or so.
 
 Go for it - how did you notice it ? Just luck ?
 
 This is why I want to get my previous CIL locking test program integrated 
 into the 'make check' somehow - or at least a separate 'make lockcheck' 
 rule

I'm getting a patch ready for this.

A few things are blocking it at the moment.  As I mentioned on IRC,
there is a licensing problem with the OCaml autoconf macros:

http://caml.inria.fr/pub/ml-archives/caml-list/2009/01/233b0823ddc93b99900d055dfa8ab3ae.en.html

This should be resolved this week.

The other thing is I found a few bugs in the code which I'm trying to
understand.  This CIL stuff is bloody complicated and obscure ...

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

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

Re: [libvirt] [PATCH] Quieten virsh schedinfo for shutoff domain

On Thu, Jan 22, 2009 at 05:49:12PM -0800, john.le...@sun.com wrote:
 +/*
 + * If it's not running, we can't help.
 + */
 +if (domain-id  0)
 +return NULL;

NACK.  Unfortunately you shouldn't return from a function without
setting an error (and you have to set an error exactly once otherwise
earlier errors get overwritten).

Perhaps if you want to silence the error, you can silence it in the
calling code, eg. in virsh, by matching on the appropriate virterror
fields, eg error-code ?

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

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

Re: [libvirt] [PATCH] Fix virsh sched-credit for xend

On Fri, Jan 23, 2009 at 10:34:14AM +, Daniel P. Berrange wrote:
 On Thu, Jan 22, 2009 at 05:49:00PM -0800, john.le...@sun.com wrote:
  # HG changeset patch
  # User john.le...@sun.com
  # Date 1232675291 28800
  # Node ID 415bfd87e0ecd7751ed6df372e82da0e3991d617
  # Parent  68e14fe50dfa88a4694bc4c7a68d2f73f41c6171
  Fix virsh sched-credit for xend
  
  Need to pass domain in the xend op for shut-down domains.
  (This also requires xend fixes, but this patch doesn't make things
  worse.)
 
 Does XenD actaully persist the schedinfo changes for inactive domains ?
 Historically we've only considered the sched tunables API to be
 relevant to active guests, and none of the drivers make any attempt
 to persist the settings for inactive domains.

I'm not sure if we make any explicit guarantees about this though, so
perhaps we should leave the decision to the drivers themselves?  Or
document how it should work in the API?

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
Read my OCaml programming blog: http://camltastic.blogspot.com/
Fedora now supports 68 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

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

Re: [libvirt] [PATCH] qemud: don't dereference NULL on failed virGetUserDirectory

On Fri, Jan 23, 2009 at 02:23:14PM +0100, Jim Meyering wrote:
 Jim Meyering j...@meyering.net wrote:
  I spotted this while merging my unix_sock_dir changes:
 ...
   char *userdir = virGetUserDirectory(NULL, uid);
  +if (userdir == NULL) {
  +/* give no diagnostic here; virGetUserDirectory reports OOM  */
 
 It can also fail due to getpwuid_r failure.
 Here's the patch with corrected comment/log:

Updated patch looks OK.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
Read my OCaml programming blog: http://camltastic.blogspot.com/
Fedora now supports 68 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

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

Re: [libvirt] [PATCH] trivial libvirt example code

On Fri, Jan 23, 2009 at 02:51:02PM -0500, Dave Allan wrote:
 The examples directory doesn't have a trivial example of how to connect  
 to a hypervisor, make a few calls, and disconnect, so I put one  
 together.  I would appreciate any suggestions on anything that I've done  
 wrong as well as suggestions for other fundamental API calls that should  
 be illustrated.

Yes, I checked this example code and it is fine.  My only comment
would be on:

 +/* virConnectOpenAuth called here with all default parameters */
 +conn = virConnectOpenAuth(NULL, virConnectAuthPtrDefault, 0);

It might be better to let people connect to a named URI.

Another possibility is to default to the test URI (test:///default)
since that (almost) always exists.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

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

Re: [libvirt] libvirt-0.5.1: problems connecting qemu:///system

On Sun, Jan 25, 2009 at 02:21:07PM +0100, Nikola Ciprich wrote:
 When I run libvirtd -f /etc/libvirt/libvirtd.conf -v -l
 and try to connect, I get:

Can you set LIBVIRT_DEBUG=1 at both the client and server ends.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/

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

Re: [libvirt] [PATCH] gnulib/tests: allow test-getaddrinfo to pass when offline

On Mon, Jan 26, 2009 at 08:30:27AM +, Mark McLoughlin wrote:
 Sometimes - yes, even in the latter part of the first decade
 of the twenty first century - one doesn't have access to the
 Internet while one is hacking.
 
 Signed-off-by: Mark McLoughlin mar...@redhat.com
 ---
  gnulib/tests/test-getaddrinfo.c |5 +
  1 files changed, 5 insertions(+), 0 deletions(-)
 
 diff --git a/gnulib/tests/test-getaddrinfo.c b/gnulib/tests/test-getaddrinfo.c
 index a887cb1..246021a 100644
 --- a/gnulib/tests/test-getaddrinfo.c
 +++ b/gnulib/tests/test-getaddrinfo.c
 @@ -64,6 +64,11 @@ int simple (char *host, char *service)
  
if (res != 0)
  {
 +  /* EAI_AGAIN is returned if no network is available. Don't fail
 +  the test merely because someone is down the country on their
 +  in-law's farm. */
 +  if (res == EAI_AGAIN)
 + return 0;
/* IRIX reports EAI_NONAME for https.  Don't fail the test
merely because of this.  */
if (res == EAI_NONAME)

+1

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

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

Re: [libvirt] PATCH: Support VNC password for QEMU guests

On Sun, Jan 25, 2009 at 09:22:53PM +0100, Radek Hladik wrote:
 Daniel P. Berrange napsal(a):
 This patch adds support for using the monitor interface to set the VNC
 password
 
   (qemu) change vnc password
   Password: 
 
 A minor tricky thing is that we can't just send the command and password
 all in one go, we must wait for the 'Password' prompt before sending the
 password.
 
 When doing this I noticed that virsh dumpxml has no way to request a
 secure XML dump (required to see the password element), nor did the
 virsh edit command set the SECURE or INACTIVE flags when changing
 the XML.
 
  qemu_conf.c   |   45 ---
  qemu_driver.c |  112 
  --
  virsh.c   |   30 ++-
  3 files changed, 131 insertions(+), 56 deletions(-)
 
 Daniel
 
 
 I tried the patch and I've found that define command do not set the 
 password as the SECURE flag is not set when defining the domain from XML 
 file. This can be very unpleasant as it effectively disables the 
 password when redefining the domain. I've tried this simple dirty hotfix
 
 diff -ur b/domain_conf.c a/domain_conf.c
 --- b/domain_conf.c 2008-12-23 14:20:02.0 +0100
 +++ a/domain_conf.c 2009-01-25 20:12:47.0 +0100
 @@ -2297,7 +2297,7 @@
  }
 
  def = virDomainDefParseNode(conn, caps, xml, root,
 -VIR_DOMAIN_XML_INACTIVE);
 +VIR_DOMAIN_XML_INACTIVE | 
 VIR_DOMAIN_XML_SECURE );
 
  cleanup:
  xmlFreeParserCtxt (pctxt);
 
 
 and it works. However there must be some better way how to do it but I 
 do not know the libvirt source code good enough to find it.

That is the correct place to put the flag - I'll include this in my
patches. We need to tell it to parse the passwd when loading the files
from disk upon daemon startup.

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] gnulib/tests: allow test-getaddrinfo to pass when offline

On Mon, Jan 26, 2009 at 08:30:27AM +, Mark McLoughlin wrote:
 Sometimes - yes, even in the latter part of the first decade
 of the twenty first century - one doesn't have access to the
 Internet while one is hacking.
 
 Signed-off-by: Mark McLoughlin mar...@redhat.com
 ---
  gnulib/tests/test-getaddrinfo.c |5 +
  1 files changed, 5 insertions(+), 0 deletions(-)
 
 diff --git a/gnulib/tests/test-getaddrinfo.c b/gnulib/tests/test-getaddrinfo.c
 index a887cb1..246021a 100644
 --- a/gnulib/tests/test-getaddrinfo.c
 +++ b/gnulib/tests/test-getaddrinfo.c
 @@ -64,6 +64,11 @@ int simple (char *host, char *service)
  
if (res != 0)
  {
 +  /* EAI_AGAIN is returned if no network is available. Don't fail
 +  the test merely because someone is down the country on their
 +  in-law's farm. */
 +  if (res == EAI_AGAIN)
 + return 0;
/* IRIX reports EAI_NONAME for https.  Don't fail the test
merely because of this.  */
if (res == EAI_NONAME)

ACK, though there's a tiny whitespace bug there.

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] Quieten virsh schedinfo for shutoff domain

On Mon, Jan 26, 2009 at 10:54:23AM +, Richard W.M. Jones wrote:

 On Thu, Jan 22, 2009 at 05:49:12PM -0800, john.le...@sun.com wrote:
  +/*
  + * If it's not running, we can't help.
  + */
  +if (domain-id  0)
  +return NULL;
 
 NACK.  Unfortunately you shouldn't return from a function without
 setting an error (and you have to set an error exactly once otherwise
 earlier errors get overwritten).
 
 Perhaps if you want to silence the error, you can silence it in the
 calling code, eg. in virsh, by matching on the appropriate virterror
 fields, eg error-code ?

The calling code isn't in virsh, it's in xen_unified. The XenStore
driver indeed cannot deal with non-running domains. I maintain this is
the correct behaviour for a sub-driver (a driver should certainly set an
error). We already have enough problems with overly eager error
reporting (try a virsh start non-existent-dom !), let's not make it
worse.

regards
john

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

Re: [libvirt] [PATCH] Fix virsh sched-credit for xend

On Mon, Jan 26, 2009 at 10:55:37AM +, Richard W.M. Jones wrote:

  Does XenD actaully persist the schedinfo changes for inactive domains ?
  Historically we've only considered the sched tunables API to be
  relevant to active guests, and none of the drivers make any attempt
  to persist the settings for inactive domains.
 
 I'm not sure if we make any explicit guarantees about this though, so
 perhaps we should leave the decision to the drivers themselves?  Or
 document how it should work in the API?

And obviously, this seems like a missing feature!

regards
john

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

Re: [libvirt] [PATCH] Fix virsh sched-credit for xend

On Mon, Jan 26, 2009 at 10:55:37AM +, Richard W.M. Jones wrote:
 On Fri, Jan 23, 2009 at 10:34:14AM +, Daniel P. Berrange wrote:
  On Thu, Jan 22, 2009 at 05:49:00PM -0800, john.le...@sun.com wrote:
   # HG changeset patch
   # User john.le...@sun.com
   # Date 1232675291 28800
   # Node ID 415bfd87e0ecd7751ed6df372e82da0e3991d617
   # Parent  68e14fe50dfa88a4694bc4c7a68d2f73f41c6171
   Fix virsh sched-credit for xend
   
   Need to pass domain in the xend op for shut-down domains.
   (This also requires xend fixes, but this patch doesn't make things
   worse.)
  
  Does XenD actaully persist the schedinfo changes for inactive domains ?
  Historically we've only considered the sched tunables API to be
  relevant to active guests, and none of the drivers make any attempt
  to persist the settings for inactive domains.
 
 I'm not sure if we make any explicit guarantees about this though, so
 perhaps we should leave the decision to the drivers themselves?  Or
 document how it should work in the API?

It isn't that simple. Tunables are not represented as part of the XML,
and if Xen is now persisting them in its config file, then any statement
we make about virDomainSetSchedularParams persisting the settings would
be unreliable. The very next call to virDomainDefineXML/CreateXML will
blow away the tunable settings that were not part of the XML.

So if we want to be able to set tunables on inactive domains, the 
virDomainSetSchedularParams() is not the right place todo it - we
can only guarentee that is is reliably usable on live domains. 

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] xm_internal.c: fix locking bug: s/Unlock/Lock/

On Mon, Jan 26, 2009 at 10:50:32AM +, Richard W.M. Jones wrote:
 On Thu, Jan 22, 2009 at 07:13:47PM +, Daniel P. Berrange wrote:
  On Thu, Jan 22, 2009 at 08:09:51PM +0100, Jim Meyering wrote:
   Merge error?
   I'll commit this in an hour or so.
  
  Go for it - how did you notice it ? Just luck ?
  
  This is why I want to get my previous CIL locking test program integrated 
  into the 'make check' somehow - or at least a separate 'make lockcheck' 
  rule
 
 I'm getting a patch ready for this.
 
 A few things are blocking it at the moment.  As I mentioned on IRC,
 there is a licensing problem with the OCaml autoconf macros:
 
 http://caml.inria.fr/pub/ml-archives/caml-list/2009/01/233b0823ddc93b99900d055dfa8ab3ae.en.html
 
 This should be resolved this week.
 
 The other thing is I found a few bugs in the code which I'm trying to
 understand.  This CIL stuff is bloody complicated and obscure ...

There are certainly some bugs in there - I'm far from convinced that I
got for/while loop handling correctly implemented. It also does not
track locks across variable assignments / aliasing.

eg if you have

   virDomainObjPtr dom = NULL;

   virDomainObjLock(driver-domains[0]);
   dom = driver-domains[0];
   
   ...do something with 'dom'

It will complain that 'dom' is used while unlocked, because it does not
track that 'driver-domains[0]' is the same as 'dom' in this context.
It wouldn't be too much more work to make that work better, but I didn't
bother at the time, because this scenario only occurred in one place
in the  libvirt code I was checking at the time.


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] kvm/virtio: Set IFF_VNET_HDR when setting up tap fds

On Mon, Jan 26, 2009 at 10:39:25AM +, Mark McLoughlin wrote:
 IFF_VNET_HDR is a tun/tap flag that allows you to send and receive
 large (i.e. GSO) packets and packets with partial checksums. Setting
 the flag means that every packet is proceeded by the same header which
 virtio uses to communicate GSO/csum metadata.

Translating this for people not familiar with the intricacies of
recent Linux networking changes ...

GSO = generic segmentation offload.  In a baremetal Linux install, the
network driver can pass the job of splitting large packets over to the
network card.  In virtualized environments, the network card is, for
example, a virtio backend running in the host.  Because the network
bridge runs entirely inside the host kernel, there are no physical
limitations on packet size as there would be if it was real ethernet,
so we can use this mechanism to pass over-sized packets to the host.
Another advantage is that you don't need to compute checksums over the
packets which are sent this way.

VNET_HDR as far as I can gather refers to the special header that
virtio_net prepends to such over-sized packets.  I'm not quite clear
if userspace has to add this header, but if so presumably that is done
inside qemu userspace(?).

Libvirt sets the flag on the socket, passes the socket by number to
qemu, and qemu needs to be able to query whether the flag was set.  So
the patch concerns itself with making sure that all the relevant bits
of this are supported.

Correct me if I'm wrong here ...

 By enabling this flag on the tap fds we create, we greatly increase
 the achievable throughput with virtio_net.
 
 However, we need to be careful to only set the flag when a) QEMU has
 support for this ABI and b) the value of the flag is queryable using
 the TUNGETIFF ioctl.
 
 It's nearly five months since kvm-74 - the first KVM release with this
 feature - was released. Up until now, we've not added libvirt support
 because there is no clean way to detect support for this in QEMU at
 runtime. A brief attempt to add a info capabilities monitor command
 to QEMU floundered. Perfect is the enemy of good enough. Probing the
 KVM version will suffice for now.
 
 Signed-off-by: Mark McLoughlin mar...@redhat.com
 ---
  src/bridge.c|   67 
 +++
  src/bridge.h|1 +
  src/qemu_conf.c |   33 +++---
  src/qemu_conf.h |1 +
  4 files changed, 93 insertions(+), 9 deletions(-)
 
 diff --git a/src/bridge.c b/src/bridge.c
 index 30afa6d..9c4ca74 100644
 --- a/src/bridge.c
 +++ b/src/bridge.c
 @@ -47,6 +47,7 @@
  #include internal.h
  #include memory.h
  #include util.h
 +#include logging.h
  
  #define MAX_BRIDGE_ID 256
  
 @@ -403,10 +404,67 @@ static int brSetInterfaceMtu(brControl *ctl,
  }
  
  /**
 + * brProbeVnetHdr:
 + * @tapfd: a tun/tap file descriptor
 + *
 + * Check whether it is safe to enable the IFF_VNET_HDR flag on the
 + * tap interface.
 + *
 + * Setting IFF_VNET_HDR enables QEMU's virtio_net driver to allow
 + * guests to pass larger (GSO) packets, with partial checksums, to
 + * the host. This greatly increases the achievable throughput.
 + *
 + * It is only useful to enable this when we're setting up a virtio
 + * interface. And it is only *safe* to enable it when we know for
 + * sure that a) qemu has support for IFF_VNET_HDR and b) the running
 + * kernel implements the TUNGETIFF ioctl(), which qemu needs to query
 + * the supplied tapfd.
 + *
 + * Returns 0 in case of success or an errno code in case of failure.
 + */
 +static int
 +brProbeVnetHdr(int tapfd)
 +{
 +#if defined(IFF_VNET_HDR)  defined(TUNGETFEATURES)  defined(TUNGETIFF)
 +unsigned int features;
 +struct ifreq dummy;
 +
 +if (ioctl(tapfd, TUNGETFEATURES, features) != 0) {
 +VIR_INFO0(_(Not enabling IFF_VNET_HDR; 
 +TUNGETFEATURES ioctl() not implemented));
 +return 0;
 +}
 +
 +if (!(features  IFF_VNET_HDR)) {
 +VIR_INFO0(_(Not enabling IFF_VNET_HDR; 
 +TUNGETFEATURES ioctl() reports no IFF_VNET_HDR));
 +return 0;
 +}
 +
 +/* The kernel will always return -1 at this point.
 + * If TUNGETIFF is not implemented then errno == EBADFD.
 + */
 +if (ioctl(tapfd, TUNGETIFF, dummy) != -1 || errno != EBADFD) {
 +VIR_INFO0(_(Not enabling IFF_VNET_HDR; 
 +TUNGETIFF ioctl() not implemented));
 +return 0;
 +}
 +
 +VIR_INFO0(_(Enabling IFF_VNET_HDR));
 +
 +return 1;
 +#else
 +VIR_INFO0(_(Not enabling IFF_VNET_HDR; disabled at build time));
 +return 0;
 +#endif
 +}
 +
 +/**
   * brAddTap:
   * @ctl: bridge control pointer
   * @bridge: the bridge name
   * @ifname: the interface name (or name template)
 + * @vnet_hdr: whether to try enabling IFF_VNET_HDR
   * @tapfd: file descriptor return value for the new tap device
   *
   * This function creates a new tap device on a bridge. @ifname can be either
 @@ -420,6

Re: [libvirt] [PATCH] Fix virsh sched-credit for xend

On Mon, Jan 26, 2009 at 11:29:01AM +, Daniel P. Berrange wrote:

 It isn't that simple. Tunables are not represented as part of the XML,

Good point, that would need to be fixed. I'd missed this.

 So if we want to be able to set tunables on inactive domains, the 
 virDomainSetSchedularParams() is not the right place todo it - we
 can only guarentee that is is reliably usable on live domains. 

No, surely we should fix the XML to represent it, and then it /would/
work. Must be better than another identical API.

(BTW I find that the API providing a bunch of unknown name/value pairs
with no indication of what they might be a little icky...)

regards
john

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

Re: [libvirt] [PATCH] Fix virsh sched-credit for xend

On Mon, Jan 26, 2009 at 11:48:42AM +, John Levon wrote:
 On Mon, Jan 26, 2009 at 11:29:01AM +, Daniel P. Berrange wrote:
 
  It isn't that simple. Tunables are not represented as part of the XML,
 
 Good point, that would need to be fixed. I'd missed this.
 
  So if we want to be able to set tunables on inactive domains, the 
  virDomainSetSchedularParams() is not the right place todo it - we
  can only guarentee that is is reliably usable on live domains. 
 
 No, surely we should fix the XML to represent it, and then it /would/
 work. Must be better than another identical API.

Historically there has been a bit of a debate about this.  It's not
clear if the XML is meant to represent static configuration about the
domain (eg. what disks it has), versus administration of the domain
(how vCPUs are pinned for example).

Is this still a real distinction we can/need to make?  Maybe time to
revisit this and allow administrative things to be added.

 (BTW I find that the API providing a bunch of unknown name/value pairs
 with no indication of what they might be a little icky...)

Yeah me too.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
Read my OCaml programming blog: http://camltastic.blogspot.com/
Fedora now supports 68 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora

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

Re: [libvirt] PATCH: Support VNC password for QEMU guests

On Mon, Jan 26, 2009 at 11:15:01AM +, Daniel P. Berrange wrote:
 On Sun, Jan 25, 2009 at 09:22:53PM +0100, Radek Hladik wrote:
  Daniel P. Berrange napsal(a):
  This patch adds support for using the monitor interface to set the VNC
  password
  
(qemu) change vnc password
Password: 
  
  A minor tricky thing is that we can't just send the command and password
  all in one go, we must wait for the 'Password' prompt before sending the
  password.
  
  When doing this I noticed that virsh dumpxml has no way to request a
  secure XML dump (required to see the password element), nor did the
  virsh edit command set the SECURE or INACTIVE flags when changing
  the XML.
  
   qemu_conf.c   |   45 ---
   qemu_driver.c |  112 
   --
   virsh.c   |   30 ++-
   3 files changed, 131 insertions(+), 56 deletions(-)
  
  Daniel
  
  
  I tried the patch and I've found that define command do not set the 
  password as the SECURE flag is not set when defining the domain from XML 
  file. This can be very unpleasant as it effectively disables the 
  password when redefining the domain. I've tried this simple dirty hotfix
  
  diff -ur b/domain_conf.c a/domain_conf.c
  --- b/domain_conf.c 2008-12-23 14:20:02.0 +0100
  +++ a/domain_conf.c 2009-01-25 20:12:47.0 +0100
  @@ -2297,7 +2297,7 @@
   }
  
   def = virDomainDefParseNode(conn, caps, xml, root,
  -VIR_DOMAIN_XML_INACTIVE);
  +VIR_DOMAIN_XML_INACTIVE | 
  VIR_DOMAIN_XML_SECURE );
  
   cleanup:
   xmlFreeParserCtxt (pctxt);
  
  
  and it works. However there must be some better way how to do it but I 
  do not know the libvirt source code good enough to find it.
 
 That is the correct place to put the flag - I'll include this in my
 patches. We need to tell it to parse the passwd when loading the files
 from disk upon daemon startup.

Actually this is not required. The password is always parsed from the
XML. The secure flag is only required when generating XML for output.

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] kvm/virtio: Set IFF_VNET_HDR when setting up tap fds

2009-01-26 Thread Mark McLoughlin

Hi Rich,

On Mon, 2009-01-26 at 11:39 +, Richard W.M. Jones wrote:
On Mon, Jan 26, 2009 at 10:39:25AM +, Mark McLoughlin wrote:
IFF_VNET_HDR is a tun/tap flag that allows you to send and receive
large (i.e. GSO) packets and packets with partial checksums. Setting
the flag means that every packet is proceeded by the same header which
virtio uses to communicate GSO/csum metadata.

Translating this for people not familiar with the intricacies of
recent Linux networking changes ...

GSO = generic segmentation offload. In a baremetal Linux install, the
network driver can pass the job of splitting large packets over to the
network card. In virtualized environments, the network card is, for
example, a virtio backend running in the host. Because the network
bridge runs entirely inside the host kernel, there are no physical
limitations on packet size as there would be if it was real ethernet,
so we can use this mechanism to pass over-sized packets to the host.
Another advantage is that you don't need to compute checksums over the
packets which are sent this way.

Yep, see also:

http://blogs.gnome.org/markmc/2008/05/28/checksums-scatter-gather-io-and-segmentation-offload/

VNET_HDR as far as I can gather refers to the special header that
virtio_net prepends to such over-sized packets. I'm not quite clear
if userspace has to add this header, but if so presumably that is done
inside qemu userspace(?).

VNET_HDR refers to the tap interface.

A tap interface is what qemu uses to inject ethernet frames into the
kernel networking stack. Normally, you just write() and read() each raw
frame with a single syscall per frame.

VNET_HDR is a flag for tap interfaces to say that frames we read() and
write() will have a struct virtio_net_hdr prepended so as to give the
kernel/qemu information about partial checksums and GSO.

We need to set this flag before we bring the interface up and add it to
a bridge. That's why libvirt has to do this rather than just leaving it
up to qemu.

Libvirt sets the flag on the socket, passes the socket by number to
qemu, and qemu needs to be able to query whether the flag was set. So
the patch concerns itself with making sure that all the relevant bits
of this are supported.

Correct me if I'm wrong here ...

You're spot on, thanks for elaborating.

Cheers,
Mark.

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

Re: [libvirt] PATCH: Support VNC password for QEMU guests

2009-01-26 Thread Radek Hladik


Daniel P. Berrange napsal(a):

On Mon, Jan 26, 2009 at 11:15:01AM +, Daniel P. Berrange wrote:
  

On Sun, Jan 25, 2009 at 09:22:53PM +0100, Radek Hladik wrote:


Daniel P. Berrange napsal(a):
  

This patch adds support for using the monitor interface to set the VNC
password

 (qemu) change vnc password
 Password: 

A minor tricky thing is that we can't just send the command and password
all in one go, we must wait for the 'Password' prompt before sending the
password.

When doing this I noticed that virsh dumpxml has no way to request a
secure XML dump (required to see the password element), nor did the
virsh edit command set the SECURE or INACTIVE flags when changing
the XML.

qemu_conf.c   |   45 ---
qemu_driver.c |  112 
--

virsh.c   |   30 ++-
3 files changed, 131 insertions(+), 56 deletions(-)

Daniel


I tried the patch and I've found that define command do not set the 
password as the SECURE flag is not set when defining the domain from XML 
file. This can be very unpleasant as it effectively disables the 
password when redefining the domain. I've tried this simple dirty hotfix


diff -ur b/domain_conf.c a/domain_conf.c
--- b/domain_conf.c 2008-12-23 14:20:02.0 +0100
+++ a/domain_conf.c 2009-01-25 20:12:47.0 +0100
@@ -2297,7 +2297,7 @@
 }

 def = virDomainDefParseNode(conn, caps, xml, root,
-VIR_DOMAIN_XML_INACTIVE);
+VIR_DOMAIN_XML_INACTIVE | 
VIR_DOMAIN_XML_SECURE );


 cleanup:
 xmlFreeParserCtxt (pctxt);


and it works. However there must be some better way how to do it but I 
do not know the libvirt source code good enough to find it.
  

That is the correct place to put the flag - I'll include this in my
patches. We need to tell it to parse the passwd when loading the files
from disk upon daemon startup.



Actually this is not required. The password is always parsed from the
XML. The secure flag is only required when generating XML for output.

Daniel
  
I am not sure about this. I've downloaded latest hourly snapshot 
yesterday, patched it with the VNC password patch and specified password 
via passwd attribute in xml. I've started the daemon and VNC required 
password - so far so good. However I've changed something other in xml 
file, reloaded the xml with virsh 'define test.xml' command and the 
machine didn't require VNC password any more. The supplied patch solved 
the issue...


Radek

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

Re: [libvirt] [PATCH] Fix virsh sched-credit for xend

On Mon, Jan 26, 2009 at 12:28:47PM +, Richard W.M. Jones wrote:

  No, surely we should fix the XML to represent it, and then it /would/
  work. Must be better than another identical API.
 
 Historically there has been a bit of a debate about this.  It's not
 clear if the XML is meant to represent static configuration about the
 domain (eg. what disks it has), versus administration of the domain
 (how vCPUs are pinned for example).

I think it's become evident that the XML represents the dynamic state of
the domain - for example, the whole code is structured around things
like the console pty being available. This would include administrative
actions, especially ones that should be persisted.

Put another way, there's nowhere else for it to go!

Yes, it would have been nice for a clear separation between the three
things (hypervisor-agnostic description of a guest 'profile', i.e.
image.rng, configuration of a domain, and runtime state of a domain),
but it cannot be changed now. Furthermore, the API does not include the
notion of persistence, sadly.

regards
john

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

[libvirt] [PATCH] Fix stderr spewage from docs/examples install

2009-01-26 Thread john . levon

# HG changeset patch
# User John Levon john.le...@sun.com
# Date 1232970961 28800
# Node ID 839f1721d6d9c2cfdbc9327814a4c4a9564ff814
# Parent  26d337992a98a00d98f83247eb113ca3d729d8ef
Fix stderr spewage from docs/examples install

*.res don't exist, so shouldn't be in the generated Makefile.am

Signed-off-by: John Levon john.le...@sun.com

diff --git a/docs/examples/Makefile.am b/docs/examples/Makefile.am
--- a/docs/examples/Makefile.am
+++ b/docs/examples/Makefile.am
@@ -20,7 +20,7 @@ install-data-local:
 install-data-local:
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
-...@install@ -m 0644 $(srcdir)/*.html $(srcdir)/*.c $(srcdir)/*.xml \
- $(srcdir)/*.xsl $(srcdir)/*.res $(DESTDIR)$(HTML_DIR)
+ $(srcdir)/*.xsl $(DESTDIR)$(HTML_DIR)
 
 EXTRA_DIST=examples.xsl index.py examples.xml
 
diff --git a/docs/examples/index.py b/docs/examples/index.py
--- a/docs/examples/index.py
+++ b/docs/examples/index.py
@@ -241,7 +241,7 @@ install-data-local:
 install-data-local:
$(mkinstalldirs) $(DESTDIR)$(HTML_DIR)
-...@install@ -m 0644 $(srcdir)/*.html $(srcdir)/*.c $(srcdir)/*.xml \\
- $(srcdir)/*.xsl $(srcdir)/*.res $(DESTDIR)$(HTML_DIR)
+ $(srcdir)/*.xsl $(DESTDIR)$(HTML_DIR)
 
 
 EXTRA_DIST= string.join(extras, ' ')

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

[libvirt] [PATCH] fix errors in virReportSystemErrorFull

While looking at removing most of the remaining uses of strerror,
I spotted problems in virReportSystemErrorFull.
  1) it would leak combined
  2) that allocated, written-into buffer wasn't even used

So, since we'd rather avoid having to allocate at all in the
error-reporting path (esp for OOM errors), I've made it automatic.

From 8550ec841b4e19049abc4b5e4a2f8a81d1824055 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 26 Jan 2009 14:59:35 +0100
Subject: [PATCH] fix errors in virReportSystemErrorFull

* src/virterror.c (virReportSystemErrorFull): Don't leak combined.
In fact, don't even attempt allocation.
Do include the result of formatted print in final diagnostic.
---
 src/virterror.c |   13 +
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/src/virterror.c b/src/virterror.c
index 0c66781..cfde1dc 100644
--- a/src/virterror.c
+++ b/src/virterror.c
@@ -1019,7 +1019,7 @@ void virReportSystemErrorFull(virConnectPtr conn,
 char systemError[1024];
 char *theerrnostr;
 const char *virerr;
-char *combined = NULL;
+char combined[2048];

 #ifdef HAVE_STRERROR_R
 #ifdef __USE_GNU
@@ -1047,10 +1047,15 @@ void virReportSystemErrorFull(virConnectPtr conn,
 errorMessage[0] = '\0';
 }

-if (virAsprintf(combined, %s: %s, errorMessage, theerrnostr)  0)
-combined = theerrnostr; /* OOM, so lets just pass the strerror info as 
best effort */
+char *err_str;
+int n = snprintf(combined, sizeof combined, %s: %s,
+ errorMessage, theerrnostr);
+err_str = (0  n  n  sizeof(combined)
+   ? combined
+   /* use the strerror info as best effort */
+   : theerrnostr);

-virerr = virErrorMsg(VIR_ERR_SYSTEM_ERROR, (errorMessage[0] ? errorMessage 
: NULL));
+virerr = virErrorMsg(VIR_ERR_SYSTEM_ERROR, (err_str[0] ? err_str : NULL));
 virRaiseError(conn, NULL, NULL, domcode, VIR_ERR_SYSTEM_ERROR, 
VIR_ERR_ERROR,
   virerr, errorMessage, NULL, -1, -1, virerr, errorMessage);
 }
--
1.6.1.1.347.g3f81d

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

[libvirt] [PATCH] * src/virterror.c (virErrorMsg): Correct indentation.

Just correct bogus indentation.

From dca64d37d946f73275fc5e8300e8cd29f34afd06 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 26 Jan 2009 14:44:22 +0100
Subject: [PATCH] * src/virterror.c (virErrorMsg): Correct indentation.

---
 src/virterror.c |   84 +++---
 1 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/src/virterror.c b/src/virterror.c
index 9732eb4..0c66781 100644
--- a/src/virterror.c
+++ b/src/virterror.c
@@ -902,66 +902,66 @@ virErrorMsg(virErrorNumber error, const char *info)
 else
 errmsg = _(Network not found: %s);
 break;
-case VIR_ERR_INVALID_MAC:
+case VIR_ERR_INVALID_MAC:
 if (info == NULL)
 errmsg = _(invalid MAC address);
 else
 errmsg = _(invalid MAC address: %s);
 break;
-case VIR_ERR_AUTH_FAILED:
+case VIR_ERR_AUTH_FAILED:
 if (info == NULL)
 errmsg = _(authentication failed);
 else
 errmsg = _(authentication failed: %s);
 break;
 case VIR_ERR_NO_STORAGE_POOL:
-if (info == NULL)
-errmsg = _(Storage pool not found);
-else
-errmsg = _(Storage pool not found: %s);
-break;
+if (info == NULL)
+errmsg = _(Storage pool not found);
+else
+errmsg = _(Storage pool not found: %s);
+break;
 case VIR_ERR_NO_STORAGE_VOL:
-if (info == NULL)
-errmsg = _(Storage volume not found);
-else
-errmsg = _(Storage volume not found: %s);
-break;
+if (info == NULL)
+errmsg = _(Storage volume not found);
+else
+errmsg = _(Storage volume not found: %s);
+break;
 case VIR_ERR_INVALID_STORAGE_POOL:
-if (info == NULL)
-errmsg = _(invalid storage pool pointer in);
-else
-errmsg = _(invalid storage pool pointer in %s);
-break;
+if (info == NULL)
+errmsg = _(invalid storage pool pointer in);
+else
+errmsg = _(invalid storage pool pointer in %s);
+break;
 case VIR_ERR_INVALID_STORAGE_VOL:
-if (info == NULL)
-errmsg = _(invalid storage volume pointer in);
-else
-errmsg = _(invalid storage volume pointer in %s);
-break;
+if (info == NULL)
+errmsg = _(invalid storage volume pointer in);
+else
+errmsg = _(invalid storage volume pointer in %s);
+break;
 case VIR_WAR_NO_STORAGE:
-if (info == NULL)
-errmsg = _(Failed to find a storage driver);
-else
-errmsg = _(Failed to find a storage driver: %s);
-break;
+if (info == NULL)
+errmsg = _(Failed to find a storage driver);
+else
+errmsg = _(Failed to find a storage driver: %s);
+break;
 case VIR_WAR_NO_NODE:
-if (info == NULL)
-errmsg = _(Failed to find a node driver);
-else
-errmsg = _(Failed to find a node driver: %s);
-break;
+if (info == NULL)
+errmsg = _(Failed to find a node driver);
+else
+errmsg = _(Failed to find a node driver: %s);
+break;
 case VIR_ERR_INVALID_NODE_DEVICE:
-if (info == NULL)
-errmsg = _(invalid node device pointer);
-else
-errmsg = _(invalid node device pointer in %s);
-break;
+if (info == NULL)
+errmsg = _(invalid node device pointer);
+else
+errmsg = _(invalid node device pointer in %s);
+break;
 case VIR_ERR_NO_NODE_DEVICE:
-if (info == NULL)
-errmsg = _(Node device not found);
-else
-errmsg = _(Node device not found: %s);
-break;
+if (info == NULL)
+errmsg = _(Node device not found);
+else
+errmsg = _(Node device not found: %s);
+break;
 }
 return (errmsg);
 }
--
1.6.1.1.347.g3f81d

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

[libvirt] [PATCH] mark a few diagnostics for translation

Avoid compile-time warnings:

From 4597152e0c4a1e4a5ee85156496a8625b5cc4f42 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 26 Jan 2009 14:54:21 +0100
Subject: [PATCH 2/3] mark a few diagnostics for translation

* src/proxy_internal.c (xenProxyCommand): Mark a diagnostic.
* src/xen_unified.c (xenUnifiedOpen, xenUnifiedAddDomainInfo): Likewise.
---
 src/proxy_internal.c |6 ++
 src/xen_unified.c|   11 +++
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/src/proxy_internal.c b/src/proxy_internal.c
index 4d8be3a..d1255ae 100644
--- a/src/proxy_internal.c
+++ b/src/proxy_internal.c
@@ -1,7 +1,7 @@
 /*
  * proxy_client.c: client side of the communication with the libvirt proxy.
  *
- * Copyright (C) 2006, 2008 Red Hat, Inc.
+ * Copyright (C) 2006, 2008, 2009 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
@@ -455,7 +455,7 @@ retry:
  */
 if ((res == NULL) || (res-version != PROXY_PROTO_VERSION) ||
 (res-len  sizeof(virProxyPacket))) {
-virProxyError(conn, VIR_ERR_INTERNAL_ERROR,
+virProxyError(conn, VIR_ERR_INTERNAL_ERROR, %s,
   _(Communication error with proxy: malformed packet\n));
 goto error;
 }
@@ -1058,5 +1058,3 @@ xenProxyDomainGetOSType(virDomainPtr domain)

 return(ostype);
 }
-
-
diff --git a/src/xen_unified.c b/src/xen_unified.c
index 1a0f007..240b8f5 100644
--- a/src/xen_unified.c
+++ b/src/xen_unified.c
@@ -261,19 +261,21 @@ xenUnifiedOpen (virConnectPtr conn, virConnectAuthPtr 
auth, int flags)

 /* Allocate per-connection private data. */
 if (VIR_ALLOC(priv)  0) {
-xenUnifiedError (NULL, VIR_ERR_NO_MEMORY, _(allocating private 
data));
+xenUnifiedError (NULL, VIR_ERR_NO_MEMORY,
+ %s, _(allocating private data));
 return VIR_DRV_OPEN_ERROR;
 }
 if (virMutexInit(priv-lock)  0) {
 xenUnifiedError (NULL, VIR_ERR_INTERNAL_ERROR,
- _(cannot initialise mutex));
+ %s, _(cannot initialise mutex));
 VIR_FREE(priv);
 return VIR_DRV_OPEN_ERROR;
 }

 /* Allocate callback list */
 if (VIR_ALLOC(cbList)  0) {
-xenUnifiedError (NULL, VIR_ERR_NO_MEMORY, _(allocating callback 
list));
+xenUnifiedError (NULL, VIR_ERR_NO_MEMORY,
+ %s, _(allocating callback list));
 virMutexDestroy(priv-lock);
 VIR_FREE(priv);
 return VIR_DRV_OPEN_ERROR;
@@ -1564,7 +1566,8 @@ xenUnifiedAddDomainInfo(xenUnifiedDomainInfoListPtr list,
 list-count++;
 return 0;
 memory_error:
-xenUnifiedError (NULL, VIR_ERR_NO_MEMORY, _(allocating domain info));
+xenUnifiedError (NULL, VIR_ERR_NO_MEMORY,
+ %s, _(allocating domain info));
 if (info)
 VIR_FREE(info-name);
 VIR_FREE(info);
--
1.6.1.1.347.g3f81d

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

Re: [libvirt] [PATCH] fix errors in virReportSystemErrorFull

On Mon, Jan 26, 2009 at 03:02:48PM +0100, Jim Meyering wrote:
 
 So, since we'd rather avoid having to allocate at all in the
 error-reporting path (esp for OOM errors), I've made it automatic.

That doesn't really matter/help the OOM reporting scenario, because
the convention for VIR_ERR_NO_MEMORY is to have a NULL fmt arg, and
the virRaiseError() method will itself fail the allocation later if
we were in a scenario where it mattered. 

  #ifdef HAVE_STRERROR_R
  #ifdef __USE_GNU
 @@ -1047,10 +1047,15 @@ void virReportSystemErrorFull(virConnectPtr conn,
  errorMessage[0] = '\0';
  }
 
 -if (virAsprintf(combined, %s: %s, errorMessage, theerrnostr)  0)
 -combined = theerrnostr; /* OOM, so lets just pass the strerror info 
 as best effort */
 +char *err_str;
 +int n = snprintf(combined, sizeof combined, %s: %s,
 + errorMessage, theerrnostr);
 +err_str = (0  n  n  sizeof(combined)
 +   ? combined
 +   /* use the strerror info as best effort */
 +   : theerrnostr);
 
 -virerr = virErrorMsg(VIR_ERR_SYSTEM_ERROR, (errorMessage[0] ? 
 errorMessage : NULL));
 +virerr = virErrorMsg(VIR_ERR_SYSTEM_ERROR, (err_str[0] ? err_str : 
 NULL));

It is actually the next line that really need to the change to use err_str - the
virErrorMsg doesn't actually care about the data in that string, only checks 
whether
it is NULL or not, so the virErrorMsg call was technically still OK, but this
next one was no good:

  virRaiseError(conn, NULL, NULL, domcode, VIR_ERR_SYSTEM_ERROR, 
 VIR_ERR_ERROR,
virerr, errorMessage, NULL, -1, -1, virerr, errorMessage);

Looking at the whole method again, I think it needs to be re-written to
something closer to this:

const char *virSystemErrorFormat(int theerrno, char *errBuf, size_t errBufLen)
{
#ifdef HAVE_STRERROR_R
#ifdef __USE_GNU
/* Annoying linux specific API contract */
return strerror_r(theerrno, errBuf, errBufLen);
#else
strerror_r(theerrno, errBuf, errBufLen);
return errBuf;
#endif
#else
/* Mingw lacks strerror_r() and its strerror() is definitely not
 * threadsafe, so safest option is to just print the raw errno
 * value - we can at least reliably  safely look it up in the
 * header files for debug purposes
 */
snprintf(errBuf, errBufLen, errno=%d, theerrno);
return errBuf;
#endif
}


void virReportSystemErrorFull(virConnectPtr conn,
  int domcode,
  int theerrno,
  const char *filename ATTRIBUTE_UNUSED,
  const char *funcname ATTRIBUTE_UNUSED,
  size_t linenr ATTRIBUTE_UNUSED,
  const char *fmt, ...)
{
char systemError[1024];
char msgDetailBuf[1024];

const char *errnoDetail = virSystemErrorFormat(theerrno, systemError, 
sizeof(systemError);
const char *msg = virErrorMsg(VIR_ERR_SYSTEM_ERROR, fmt);
const char *msgDetail;

if (fmt) {
va_list args;

va_start(args, fmt);
vsnprintf(msgDetailBuf, sizeof(msgDetailBuf)-1, fmt, args);
va_end(args);

strncat(msgDetailBuf, errnoDetail, 
(sizeof(msgDetailBuf)-1)-strlen(msgDetail));
msgDetail = msgDetailBuf;
} else {
msgDetail = errnoDetail;
}

virRaiseError(conn, NULL, NULL, domcode, VIR_ERR_SYSTEM_ERROR, 
VIR_ERR_ERROR,
  msg, msgDetail, NULL, -1, -1, msg, msgDetail);
}

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] * src/virterror.c (virErrorMsg): Correct indentation.

On Mon, Jan 26, 2009 at 03:03:59PM +0100, Jim Meyering wrote:
 Just correct bogus indentation.
 
 From dca64d37d946f73275fc5e8300e8cd29f34afd06 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 26 Jan 2009 14:44:22 +0100
 Subject: [PATCH] * src/virterror.c (virErrorMsg): Correct indentation.

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] mark a few diagnostics for translation

On Mon, Jan 26, 2009 at 03:05:05PM +0100, Jim Meyering wrote:
 Avoid compile-time warnings:
 
 From 4597152e0c4a1e4a5ee85156496a8625b5cc4f42 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Mon, 26 Jan 2009 14:54:21 +0100
 Subject: [PATCH 2/3] mark a few diagnostics for translation

Any calls which have a VIR_ERR_NO_MEMORY can just be replaced by
a call to virReportOOMError(). Ack to the other non-OOM error msg
changes.

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: [Patch][RFC] Fine grained access control in libvirt by rbac (0/3)

2009-01-26 Thread Konrad Eriksson1

Yes, we're looking into adding similar form of access control in libvirt.

The approach we're looking at is to inject AC as a module that intercepts 
calls from the libvirt core (libvirt.c) to the drivers.
Reason:
* AC module can be loaded/unloaded on the fly without need to recompile 
(can support several different AC-modules and load the appropriate one 
during connect).
* Making use of (semi-stable) API between libvirt core and drivers (also 
hypervisor/storage/network driver independent)
   * Being in the call-path also enables AC-module to alter return values 
(such as filtering lists of VMs/NETs/Storages based on access rights)
* Minimal code changes in existing libvirt code (basically a one-liner in 
libvirt.c to inject AC)

What still is an issue is how to correctly get the identity of the user, 
especially over remote connection.
I guess you have the same problem? (you're only allowing local usage for 
now).
The best way would be to link some user-auth data with the virConnectPtr, 
but becomes a bit trickier when authentication is done prior (like in 
remote case) to virConnectOpen.

You implemented your own RBAC language to describe the AC-policies.
Have you looked at possibility to link with already existing RBAC 
mechanisms for Linux (like SELinux or maybe simpler AC-libs)?


Freundliche Grüsse / Best regards


Konrad Eriksson
Trusted Computing / Security  Assurance

Email: k...@zurich.ibm.com
Phone: +41 (0)44 724 84 28 

IBM Zurich Research Laboratory
www.zurich.ibm.com
Saeumerstrasse 4
8803 Rueschlikon
Switzerland 




From:
Syunsuke HAYASHI syuns...@jp.fujitsu.com
To:
libvir-list@redhat.com
Cc:
Konrad Eriksson1 k...@zurich.ibm.com, berra...@redhat.com, Atsushi SAKAI 
sak...@jp.fujitsu.com, INAKOSHI Hiroya inakoshi.hir...@jp.fujitsu.com
Date:
01/26/2009 11:25 AM
Subject:
[Patch][RFC] Fine grained access control in libvirt by rbac (0/3)



The series of patches introduces a fine grained access control to
libvirt.  They enable libvirt to enforce users what operations to invoke
in role-based way.  Our team found that Konrad and Daniel have similar
interest to ours.  Comments and suggestions are very welcome.

Patches:
 - Embedding hooks in libvirt (1/3)
 - Access control library (2/3)
 - Example policy files (3/3)







image/gif

smime.p7s
Description: S/MIME Cryptographic Signature
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] PATCH: Support VNC password for QEMU guests

On Tue, Jan 20, 2009 at 11:08:56PM +, Daniel P. Berrange wrote:
 This patch adds support for using the monitor interface to set the VNC
 password
 
   (qemu) change vnc password
   Password: 
 
 A minor tricky thing is that we can't just send the command and password
 all in one go, we must wait for the 'Password' prompt before sending the
 password.
 
 When doing this I noticed that virsh dumpxml has no way to request a
 secure XML dump (required to see the password element), nor did the
 virsh edit command set the SECURE or INACTIVE flags when changing
 the XML.
 
  qemu_conf.c   |   45 ---
  qemu_driver.c |  112 
 --
  virsh.c   |   30 ++-
  3 files changed, 131 insertions(+), 56 deletions(-)

This update also makes it possible to set a global default password
for all QEMU vms in /etc/libvirt/qemu.conf. This mirrors a similar
option XenD has. A per-VM specified password always takes priority

 qemud/Makefile.am|2 
 qemud/libvirtd_qemu.aug  |1 
 qemud/test_libvirtd.aug  |   18 ++
 qemud/test_libvirtd_qemu.aug |   33 +++-
 src/qemu.conf|   11 
 src/qemu_conf.c  |   58 ++---
 src/qemu_conf.h  |1 
 src/qemu_driver.c|  116 +--
 src/uml_driver.c |4 +
 src/virsh.c  |   30 +++
 10 files changed, 204 insertions(+), 70 deletions(-)

Daniel

diff --git a/qemud/Makefile.am b/qemud/Makefile.am
--- a/qemud/Makefile.am
+++ b/qemud/Makefile.am
@@ -246,6 +246,8 @@ libvirtd.init: libvirtd.init.in
 check-local:
test -x '$(AUGPARSE)' \
   '$(AUGPARSE)' -I $(srcdir) $(srcdir)/test_libvirtd.aug || :
+   test -x '$(AUGPARSE)' \
+  '$(AUGPARSE)' -I $(srcdir) $(srcdir)/test_libvirtd_qemu.aug || :
 
 else
 
diff --git a/qemud/libvirtd_qemu.aug b/qemud/libvirtd_qemu.aug
--- a/qemud/libvirtd_qemu.aug
+++ b/qemud/libvirtd_qemu.aug
@@ -26,6 +26,7 @@ module Libvirtd_qemu =
  | bool_entry vnc_tls
  | str_entry vnc_tls_x509_cert_dir
  | bool_entry vnc_tls_x509_verify
+ | str_entry vnc_password
 
(* Each enty in the config is one of the following three ... *)
let entry = vnc_entry
diff --git a/qemud/test_libvirtd.aug b/qemud/test_libvirtd.aug
--- a/qemud/test_libvirtd.aug
+++ b/qemud/test_libvirtd.aug
@@ -259,6 +259,15 @@ max_requests = 20
 # this should be a small fraction of the global max_requests
 # and max_workers parameter
 max_client_requests = 5
+
+# Logging level:
+log_level = 4
+
+# Logging outputs:
+log_outputs=\4:stderr\
+
+# Logging filters:
+log_filters=\a\
 
 
test Libvirtd.lns get conf =
@@ -525,3 +534,12 @@ max_client_requests = 5
 { #comment = this should be a small fraction of the global 
max_requests }
 { #comment = and max_workers parameter }
 { max_client_requests = 5 }
+   { #empty }
+{ #comment = Logging level: }
+{ log_level = 4 }
+   { #empty }
+{ #comment = Logging outputs: }
+{ log_outputs = 4:stderr }
+   { #empty }
+{ #comment = Logging filters: }
+{ log_filters = a }
diff --git a/qemud/test_libvirtd_qemu.aug b/qemud/test_libvirtd_qemu.aug
--- a/qemud/test_libvirtd_qemu.aug
+++ b/qemud/test_libvirtd_qemu.aug
@@ -50,14 +50,16 @@ vnc_tls_x509_cert_dir = \/etc/pki/libvi
 #
 vnc_tls_x509_verify = 1
 
-# Logging level:
-log_level = 4
 
-# Logging outputs:
-log_outputs=4:stderr
-
-# Logging filters:
-log_filters=
+# The default VNC password. Only 8 letters are significant for
+# VNC passwords. This parameter is only used if the per-domain
+# XML config does not already provide a password. To allow
+# access without passwords, leave this commented out. An empty
+# string will still enable passwords, but be rejected by QEMU
+# effectively preventing any use of VNC. Obviously change this
+# example here before you set this
+#
+vnc_password = \XYZ12345\
 
 
test Libvirtd_qemu.lns get conf =
@@ -110,9 +112,14 @@ log_filters=
 { #comment = certificate signed by the CA in 
/etc/pki/libvirt-vnc/ca-cert.pem }
 { #comment =  }
 { vnc_tls_x509_verify = 1 }
-{ #comment = Logging level: }
-{ log_level = 4 }
-{ #comment = Logging outputs: }
-{ log_outputs = 4:stderr }
-{ #comment = Logging filters }
-{ log_filters =  }
+{ #empty }
+{ #empty }
+{ #comment = The default VNC password. Only 8 letters are significant for }
+{ #comment = VNC passwords. This parameter is only used if the per-domain }
+{ #comment = XML config does not already provide a password. To allow }
+{ #comment = access without passwords, leave this commented out. An empty }
+{ #comment = string will still enable passwords, but be rejected by QEMU }
+{ #comment = effectively preventing any use of VNC. Obviously change this }
+{ #comment = example here before you set this }
+{

[libvirt] Re: [Patch][RFC] Fine grained access control in libvirt by rbac (0/3)

On Mon, Jan 26, 2009 at 04:19:14PM +0100, Konrad Eriksson1 wrote:
 Yes, we're looking into adding similar form of access control in libvirt.
 
 The approach we're looking at is to inject AC as a module that intercepts 
 calls from the libvirt core (libvirt.c) to the drivers.
 Reason:
 * AC module can be loaded/unloaded on the fly without need to recompile 
 (can support several different AC-modules and load the appropriate one 
 during connect).
 * Making use of (semi-stable) API between libvirt core and drivers (also 
 hypervisor/storage/network driver independent)
* Being in the call-path also enables AC-module to alter return values 
 (such as filtering lists of VMs/NETs/Storages based on access rights)
 * Minimal code changes in existing libvirt code (basically a one-liner in 
 libvirt.c to inject AC)
 
 What still is an issue is how to correctly get the identity of the user, 
 especially over remote connection.
 I guess you have the same problem? (you're only allowing local usage for 
 now).

There are lots of scenarios to consider

 - Direct local API usage. You have the PID, UID  GID of the process
 - Local usage via libvirtd + UNIX sockets. You can get the PID, UID  GID
   of the client end using the SCM_CREDENTIALS message. (see man 7 unix)
 - Remote usage via libvirtd + TCP sockets. Depending on the security 
   encryption settings you may have a SASL username, or a x509 certificate
   CNAME, or both, or neither.
 - Local usage via libvirtd + UNIX sockets + libvirt-qpid. The PID, UID
GID of the client end aren't particularly usage, since libvirt-qpid
   is just a demon running as root, which accepts calls on behalf of many
   remote apps. Does QPid provide any identifying info about the entity
   which put the message on the queue.
 - Remote usage with IPSec ?

So there are multiple identifying credentials, in multiple formats, and
need some way to associate this information with a connection.

Applying RBAC to local (non-Daemon) API usage has clear limitations - if
the user running virsh (or equiv) has direct access to the system, then
they could trivially just replace the real virsh with their own virsh
without RBAC. So RBAC usage in the non-Daemon context is only useful 
if the user does not have direct access to the ssytem (eg, virsh is being
invoked on their behalf by another tool, or a constrained environment
where its guarenteed they can't provide their own libraries/binaries.

 The best way would be to link some user-auth data with the virConnectPtr, 
 but becomes a bit trickier when authentication is done prior (like in 
 remote case) to virConnectOpen.

The key question is do we need to pass the client/callers identity at
the time we create the connection object, or is it sufficient to
provide it after the fact with a call like

   virConnectAddSecParam(VIR_SECPARAM_UNIX_UID, getuid());
   virConnectAddSecParam(VIR_SECPARAM_UNIX_GID, getgid());
   virConnectAddSecParam(VIR_SECPARAM_USERNAME, saslUsername);
   virConnectAddSecParam(VIR_SECPARAM_X509_CNAME, saslUsername);

Or do we need to provide this info via some form of callback mechanism,
perhaps via the exiting virConnectCredential struct, which is curently
not used on the server end of remote connections.

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] Fix virsh sched-credit for xend

On Mon, Jan 26, 2009 at 01:21:46PM +, John Levon wrote:
 On Mon, Jan 26, 2009 at 12:28:47PM +, Richard W.M. Jones wrote:
 
   No, surely we should fix the XML to represent it, and then it /would/
   work. Must be better than another identical API.
  
  Historically there has been a bit of a debate about this.  It's not
  clear if the XML is meant to represent static configuration about the
  domain (eg. what disks it has), versus administration of the domain
  (how vCPUs are pinned for example).
 
 I think it's become evident that the XML represents the dynamic state of
 the domain - for example, the whole code is structured around things
 like the console pty being available. This would include administrative
 actions, especially ones that should be persisted.
 
 Put another way, there's nowhere else for it to go!
 
 Yes, it would have been nice for a clear separation between the three
 things (hypervisor-agnostic description of a guest 'profile', i.e.
 image.rng, configuration of a domain, and runtime state of a domain),
 but it cannot be changed now. Furthermore, the API does not include the
 notion of persistence, sadly.

Yup, I totally agree with you.

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top

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

[libvirt] PATCH: Fix remote driver RPC recv handling

I noticed a odd error message randomly appearing from virsh after all 
commands had been run

  # virsh dominfo VirtTest
  Id: -
  Name:   VirtTest
  UUID:   82038f2a-1344-aaf7-1a85-2a7250be2076
  OS Type:hvm
  State:  shut off
  CPU(s): 3
  Max memory: 256000 kB
  Used memory:256000 kB
  Autostart:  disable

  libvir: Remote error : server closed connection

It turns out that inthe for(;;) loop where I'm procesing incoming data,
it was forgetting to break out of the loop when a completed RPC call
had been received. Most of the time this wasn't  problem, because there'd
rarely be more data following, so it'd get EAGAIN next iteration  quit
the loop. When shutting down though, you'd occassionally see the SIGHUP
from read() before you get an EAGAIN, causing this error to be raised
even though the RPC call had been successfully received.

In addition, if there was a 2nd RPC reply already pending, we'd read and
loose some of its data. Though I never saw this happen, it is a definite
theoretical possibility, particularly over a TCP link with bad latency
or fragementation or bursty traffic.

Daniel

diff -r e582072116a3 src/remote_internal.c
--- a/src/remote_internal.c Mon Jan 26 16:21:42 2009 +
+++ b/src/remote_internal.c Mon Jan 26 17:02:15 2009 +
@@ -6135,12 +6135,27 @@ processCallRecv(virConnectPtr conn, stru
 if (priv-bufferOffset == priv-bufferLength) {
 if (priv-bufferOffset == 4) {
 ret = processCallRecvLen(conn, priv, in_open);
+if (ret  0)
+return -1;
+
+/*
+ * We'll carry on around the loop to immediately
+ * process the message body, because it has probably
+ * already arrived. Worst case, we'll get EAGAIN on
+ * next iteration.
+ */
 } else {
 ret = processCallRecvMsg(conn, priv, in_open);
 priv-bufferOffset = priv-bufferLength = 0;
-}
-if (ret  0)
-return -1;
+/*
+ * We've completed one call, so return even
+ * though there might still be more data on
+ * the wire. We need to actually let the caller
+ * deal with this arrived message to keep good
+ * response, and also to correctly handle EOF.
+ */
+return ret;
+}
 }
 }
 }


-- 
|: 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] mark a few diagnostics for translation