[libvirt] reload libvirtd.conf has a little problem.

2010-03-01 Thread Osier Yang

hi
When libvirtd is running,  I changed unix_sock_ro_perms=0777 into 
unix_sock_ro_perms=0755, and then reload the configuration with:

 kill -SIGHUP `pidof libvirtd`  or service libvirtd reload

 the permission of /var/lib/libvirt/libvirt-sock-ro still is 0777, 
but not 0755.


 and after service libvirtd restart,  the permission is changed 
into 0755.


 think it's a little problem.

 libvirt version: libvirt-0.7.6-1.fc12.x86_64

Thanks and Regards
osier

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


Re: [libvirt] [PATCH] build: avoid warning about unused variables

2010-03-01 Thread Daniel Veillard
On Thu, Feb 25, 2010 at 02:28:10PM +0100, Jim Meyering wrote:
 
 
 From 8ddff3e6cdccc2b4289509c6941b43f2ddf8e643 Mon Sep 17 00:00:00 2001
 From: Jim Meyering meyer...@redhat.com
 Date: Thu, 25 Feb 2010 14:19:33 +0100
 Subject: [PATCH] build: avoid warning about unused variables
 
 * tools/virsh.c (cmdCPUBaseline): Remove declarations of unused
 variables, p and cur.
 ---
  tools/virsh.c |3 +--
  1 files changed, 1 insertions(+), 2 deletions(-)
 
 diff --git a/tools/virsh.c b/tools/virsh.c
 index 5fdbbe5..89eefcf 100644
 --- a/tools/virsh.c
 +++ b/tools/virsh.c
 @@ -7048,12 +7048,11 @@ cmdCPUBaseline(vshControl *ctl, const vshCmd *cmd)
  int found;
  int ret = TRUE;
  char *buffer;
 -char *p;
  char *result = NULL;
  const char **list = NULL;
  unsigned int count = 0;
  xmlDocPtr doc = NULL;
 -xmlNodePtr node_list, cur;
 +xmlNodePtr node_list;
  xmlXPathContextPtr ctxt = NULL;
  xmlSaveCtxtPtr sctxt = NULL;
  xmlBufferPtr buf = NULL;

  Heh, thanks, I'm surprized I didn't see it, maybe because I built
without optimization when developping the patch...

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] reload libvirtd.conf has a little problem.

2010-03-01 Thread Daniel P. Berrange
On Mon, Mar 01, 2010 at 05:30:21PM +0800, Osier Yang wrote:
 hi
 When libvirtd is running,  I changed unix_sock_ro_perms=0777 into 
 unix_sock_ro_perms=0755, and then reload the configuration with:
  kill -SIGHUP `pidof libvirtd`  or service libvirtd reload
 
  the permission of /var/lib/libvirt/libvirt-sock-ro still is 0777, 
 but not 0755.
 
  and after service libvirtd restart,  the permission is changed 
 into 0755.
 
  think it's a little problem.
 
  libvirt version: libvirt-0.7.6-1.fc12.x86_64

libvirtd never attempts to reload /etc/libvirt/libvirtd.conf while
running. The SIGHUP only triggers a reload of the hypervisor drivers'
guest/network configuration files.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] Hypervisor raw options

2010-03-01 Thread Daniel P. Berrange
On Fri, Feb 26, 2010 at 07:47:20PM +0100, Daniel Dehennin wrote:
 Hello,
 
 I wonder if it's possible to add hypervisor pass-through options, I add
 some usage of kvm -startdate and need to use a script which call kvm
 with the desired option.
 
 Something like:
 
 emulator/usr/bin/kvm/emulator
 emulator-options
 option-option1/option
 option-option2/option
 /emulator-options
 
 The several option/ will be passed to the emulator/ command.

The goal of the libvirt XML format is to provide a representation that is
consistent across all hypervisors, so that regardless of whether using
Xen, KVM, or VMWare an config parameter can be encoded that same way.

If we were to support your suggestion, every hypervisor in libvirt would
end up with a different XML configuration. In addition KVM / QEMU have
a history of renaming/breaking command line options, so there's no way of
guarenteeing that '-foo' from QEMU 0.9.1 still works with QEMU 0.10.0 or
vica-verca.

If you have specific KVM arguments that libvirt does not yet support then
please either file a RFE bug report with details, or raise the issue on
this mailing list

WRT your example of -startdate, I posted patches for supporting that
last week

  http://www.redhat.com/archives/libvir-list/2010-February/msg00104.html

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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/9] Add QEMU driver support for job info on migration ops

2010-03-01 Thread Daniel P. Berrange
On Fri, Feb 26, 2010 at 06:51:05PM +0100, Daniel Veillard wrote:
 On Thu, Feb 18, 2010 at 03:56:10PM +, Daniel P. Berrange wrote:
  Introduce support for  virDomainGetJobInfo in the QEMU driver. This
  allows for monitoring of any API that uses the 'info migrate' monitor
  command. ie virDomainMigrate, virDomainSave and virDomainCoreDump
  
  Unfortunately QEMU does not provide a way to monitor incoming migration
  so we can't wire up virDomainRestore yet.
  
  The virsh tool gets a new command 'domjobinfo' to query status
  
  * src/qemu/qemu_driver.c: Record virDomainJobInfo and start time
in qemuDomainObjPrivatePtr objects. Add generic shared handler
for calling 'info migrate' with all migration based APIs.
  * src/qemu/qemu_monitor_text.c: Fix parsing of 'info migration' reply
  * tools/virsh.c: add new 'domjobinfo' command to query progress
  ---
   src/qemu/qemu_driver.c   |  208 
  +++---
   src/qemu/qemu_monitor_text.c |7 +-
   tools/virsh.c|  129 ++
   3 files changed, 288 insertions(+), 56 deletions(-)
  
  diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
  index a6dc4f9..b245eb2 100644
  --- a/src/qemu/qemu_driver.c
  +++ b/src/qemu/qemu_driver.c
  @@ -87,6 +87,8 @@ struct _qemuDomainObjPrivate {
   int jobActive; /* Non-zero if a job is active. Only 1 job is allowed 
  at any time
   * A job includes *all* monitor commands, even those 
  just querying
   * information, not merely actions */
  +virDomainJobInfo jobInfo;
  +unsigned long long jobStart;
   
   qemuMonitorPtr mon;
   virDomainChrDefPtr monConfig;
  @@ -329,6 +331,8 @@ static int qemuDomainObjBeginJob(virDomainObjPtr obj)
   }
   }
   priv-jobActive = 1;
  +priv-jobStart = (now.tv_sec * 1000ull) + (now.tv_usec / 1000);
  +memset(priv-jobInfo, 0, sizeof(priv-jobInfo));
   
   return 0;
   }
  @@ -373,6 +377,8 @@ static int qemuDomainObjBeginJobWithDriver(struct 
  qemud_driver *driver,
   }
   }
   priv-jobActive = 1;
  +priv-jobStart = (now.tv_sec * 1000ull) + (now.tv_usec / 1000);
  +memset(priv-jobInfo, 0, sizeof(priv-jobInfo));
 
   Hum, I though the time was measured in second in the job strcture, why
 keep a millisecond timestamp here ? Maybe seconds are too coarse in the
 API anyway

Yeah, the public API comment was wrong - we were passing milliseconds out
to the client app, and virsh is processing it as milliseconds.

 
   virDomainObjUnlock(obj);
   qemuDriverLock(driver);
  @@ -395,6 +401,8 @@ static int ATTRIBUTE_RETURN_CHECK 
  qemuDomainObjEndJob(virDomainObjPtr obj)
   qemuDomainObjPrivatePtr priv = obj-privateData;
   
   priv-jobActive = 0;
  +priv-jobStart = 0;
  +memset(priv-jobInfo, 0, sizeof(priv-jobInfo));
   virCondSignal(priv-jobCond);
   
   return virDomainObjUnref(obj);
  @@ -3919,6 +3927,96 @@ cleanup:
   }
   
   
  +static int
  +qemuDomainWaitForMigrationComplete(struct qemud_driver *driver, 
  virDomainObjPtr vm)
  +{
  +int ret = -1;
  +int status;
  +unsigned long long memProcessed;
  +unsigned long long memRemaining;
  +unsigned long long memTotal;
  +qemuDomainObjPrivatePtr priv = vm-privateData;
  +
  +priv-jobInfo.type = VIR_DOMAIN_JOB_UNBOUNDED;
  +
  +while (priv-jobInfo.type == VIR_DOMAIN_JOB_UNBOUNDED) {
  +/* Poll every 1/2 second for progress  to allow cancellation */
 
   that's way larger than an human perception window so from
 an UI perspective the user will percieve the time between
 the request and when it takes effect, probably not a big deal
 but could be refined in the future.

Migration still starts immediately, we merely have a 1/2 second delay
for detecting its completion. We need to improve this regardless though
since migraiton is supposed to be seemless !

 
  +struct timespec ts = { .tv_sec = 0, .tv_nsec = 500 * 1000ull };
  +struct timeval now;
  +int rc;
  +
  +qemuDomainObjEnterMonitorWithDriver(driver, vm);
 
   ACK,
 

Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] reload libvirtd.conf has a little problem.

2010-03-01 Thread Osier Yang

hmmm, I see.  Thanks.

Regards
osier

Daniel P. Berrange wrote:

On Mon, Mar 01, 2010 at 05:30:21PM +0800, Osier Yang wrote:
  

hi
When libvirtd is running,  I changed unix_sock_ro_perms=0777 into 
unix_sock_ro_perms=0755, and then reload the configuration with:

 kill -SIGHUP `pidof libvirtd`  or service libvirtd reload

 the permission of /var/lib/libvirt/libvirt-sock-ro still is 0777, 
but not 0755.


 and after service libvirtd restart,  the permission is changed 
into 0755.


 think it's a little problem.

 libvirt version: libvirt-0.7.6-1.fc12.x86_64



libvirtd never attempts to reload /etc/libvirt/libvirtd.conf while
running. The SIGHUP only triggers a reload of the hypervisor drivers'
guest/network configuration files.

Regards,
Daniel
  


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


Re: [libvirt] [PATCH] Free the macvtap mode string

2010-03-01 Thread Daniel Veillard
On Fri, Feb 26, 2010 at 01:03:56PM -0500, Stefan Berger wrote:
 I forgot to free the macvtap mode string. This fixes it.
 
 Signed-off-by: Stefan Berger stef...@us.ibm.com
 

 Index: libvirt-macvtap/src/conf/domain_conf.c
 ===
 --- libvirt-macvtap.orig/src/conf/domain_conf.c
 +++ libvirt-macvtap/src/conf/domain_conf.c
 @@ -1974,6 +1974,7 @@ cleanup:
  VIR_FREE(type);
  VIR_FREE(internal);
  VIR_FREE(devaddr);
 +VIR_FREE(mode);
  
  return def;

  Ah, right, ACK, pushing it,

   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


[libvirt] and for LXC?

2010-03-01 Thread Mihamina Rakotomandimby
Manao ahoana, Hello, Bonjour,

Is there some plan/schedule to make libvirt LXC friendly?
Or is it already?

Misaotra, Thanks, Merci.

-- 
   Architecte Informatique chez Blueline/Gulfsat:
Administration Systeme, Recherche  Developpement
+261 34 29 155 34 / +261 33 11 207 36

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


Re: [libvirt] Problems with attach-device/detach-device using libvirt 0.7.6

2010-03-01 Thread Daniel P. Berrange
On Fri, Feb 26, 2010 at 04:24:11PM +0100, Rolf Eike Beer wrote:
 Hi,
 
 first a short summary of what the situation is:
 
 I'm running qemu 0.12.2, libvirt 0.7.6 on a E5520 based machine with kernel 
 2.6.33. I also tried 2.6.32.7 as I was using that before and still had the 
 old 
 kernel around, but kernel version doesn't matter AFAICT. The host is a gentoo 
 AMD64 installation, the client is SuSE 11.0 (both 32 and 64 bit tested).
 
 This system is our machine to run automated PCIe device tests on, so the idea 
 is to move PCIe devices from the host to different vms, run some testcases 
 there and then move them away again. This has worked before (I'm not 
 absolutely sure, but I would bet a bit of money that it was libvirt 0.7.5).
 
 So, what I'm going to do is simply
 
 virsh # attach-device suse11.0-AMD64 dev-pci-AD1-CL.xml
 Device attached successfully
 
 virsh # detach-device suse11.0-AMD64 dev-pci-AD1-CL.xml
 Device detached successfully
 
 This is just was I expect and what I now get with the two patches attached. 
 The original situation was somewhat different (i.e. broken):
 
 -first I got complains in the log stating:
 
 tried to create id (null) twice for device
 
 Well, adding two devices with the same id is bad, and if that id is a NULL 
 pointer anyway IMHO it's better to not pass the parameter at all. That's what 
 the null-pci-id patch does.

The goal of libvirt when using QEMU = 0.12 is that 'id=' parameter is set
for all devices. This enables libvirt reliably identify devices when talking
to QEMU. Since you were seeing '(null)' there must be a place in the code
where we've forgotton to generate the device alias name for hotplug. We should
fix that, rather than hiding the problem.

 
 -next, detaching failed. I found out that the command sent to qemu was  
 pci_del pci_addr=fc67add0:7f41:fe38b65a which is obviously bogus. After some 
 hours of digging through the command stack up and down I found that it was 
 the 
 memcpy addressed in the pci_memcpy patch.
 
 When the device is attached qemu just replies with an empty string. 
 Afterwards 
 from the guestAddr the address of the PCI device in the client is copied back 
 to the info structure. Too bad that this structure was never initialized to 
 anything useful (at least not for me). When I kick out the memcpy() 
 everything 
 works fine.

Ok, I see what's wrong here. The code has two ways of attaching devices

if (qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE)
ret = qemuMonitorAddDevice(priv-mon, devstr);
else
ret = qemuMonitorAddPCIHostDevice(priv-mon,
  hostdev-source.subsys.u.pci,
  guestAddr);

With the qemuMonitorAddDevice() method, libvirt decides the PCi address ahead
of time. In the qemuMonitorAddPCIHostDevice() method, QEMU decides the PCI
addresses  tells us about it. Thus we only want todo the memcpy() bit in the
'else' case, never the 'if' case. 

 Note: when the client PCI id is passed to qemu anyway it would be cool if 
 that 
 could be specified in the device XML. For USB devices, too.

You already can supply the PCI address in the XML - if you run 'virsh dumpxml'
on your running guest you'll see the syntax for PCI addresses. For USB devices,
the device IDs on the bus are assigned by the guest OS when it activates the
USB devices, so I don't think there's any way to allow user specification of
that


Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] libvirt/dnsmasq integration.

2010-03-01 Thread Daniel P. Berrange
On Wed, Feb 24, 2010 at 02:34:53PM +, Simon Kelley wrote:
 As the principal maintainer of dnsmasq, I'm seeing increasing reports of 
 problems on systems which run both dnsmasq and libvirt. I'm fairly sure 
 I understand what's going on in these cases, and I have a few proposals 
 for changes in libvir and dnsmasq that should fix things.

Thanks for starting this topic - it would certainly be nice if we can come
up with a solution that has better inter-operability  fewer surprises for
administrators.

 The problem is that libvirt runs a private instance of dnsmasq: on 
 machines which are also running a system dnsmasq daemon, this can 
 cause problems.
 
 Some background: dnsmasq can run in two modes.
 
 Default mode: dnsmasq binds the wildcard address and does network magic
 to determine which interface request packets actually come from, so that
 the results can be sent back with the correct source address. This has
 the advantage that network interfaces can come and go and change IP
 address and dnsmasq will keep working. It's possible to restrict dnsmasq
 to only reply to requests on some interfaces; requests from other
 interfaces will be read by dnsmasq and then silently dropped. Telling
 dnsmasq to use an interface which doesn't exist but might in the future
 will result in a logged warning, but dnsmasq will still start and when
 the interface comes up it will work.
 
 Bind-interfaces mode: This is the traditional way to do UDP servers. At
 startup dnsmasq enumerates all the extant interfaces and then opens a
 socket for each one, listening on the interfaces's IP address.
 Interfaces may be skipped if excluded by the --interface and
 --except-interface flags, and any interface specified in --interface
 which doesn't exist at start-up will generate a fatal error.

Yep, I remember we hit that fatal error in libvirt, when we create our
bridge device  then launched dnsmasq, sometimes dnsmasq would exit
with an error because the bridge device wasn't visible in userspace
yet. Thus we use bind-interfaces mode, but instead of using the flag
--interface=virbr0, we switched to --listen-address=IP-of-VIRBR0

I imagine you've already seen this, but as an example of the ARGV that
libvirt generates for its dnsmasq instances:

  /usr/sbin/dnsmasq \
   --strict-order \
   --bind-interfaces \
   --pid-file=/var/run/libvirt/network/default.pid \
   --conf-file=  \
   --listen-address 192.168.122.1 \
   --except-interface lo \
   --dhcp-range 192.168.122.2,192.168.122.254 \
   --dhcp-lease-max=253


 In almost all cases, default mode is better: --bind-interfaces is only
 there to cope with old platforms which don't support enough socket
 options to do default mode.
 
 The only time when --bind-interfaces works better is when it's desirable
 to run more than one instance of dnsmasq or have dnsmasq co-exist with 
 another DNS server. This is not possible in default mode, but it does 
 work in bind-interfaces mode, providing than _all_ instances of dnsmasq 
 are in bind-interfaces mode, and that they listen on a disjoint set of 
 interfaces.

Yes, for want of any alternative, we currently recommend users with a
system instance of dnsmasq to use bind-interfaces, and either --interface
or --listen-address

  http://wiki.libvirt.org/page/Libvirtd_and_dnsmasq


 Therefore, to allow multiple dnsmasq instances libvirt's private dnsmasq 
 instance is started in bind-interfaces mode: that forces one of the 
 dnsmasq instances to do bind-interfaces. Many of the Linux distibution 
 dnsmasq packages have now implemented an /etc/dnsmasq.d directory where 
 configuration fragments can be dropped. Their libvirt packages are 
 putting a file there which contains a bind-interfaces command, so that 
 the system dnsmasq is automatically forced into the same mode, and the 
 two can co-exist.
 
 This works, sort-of, but there some disadvantages. Installing libvirt 
 drops the configuration change for the system dnsmasq, but the packages 
 frequently don't restart the system daemon, so that things transiently 
 fail until everything has rebooted. Much worse, the system dnsmasq is 
 forced into bind-interfaces mode and then service to transient 
 interfaces (usb, ad-hoc wifi) no longer works, or, because those 
 interfaces are mentioned in the dnsmasq configuration, dnsmasq now fails 
 at start-up when the interfaces don't exist.

Yes, this is rather a pain. Aside from the scheme you propose later,
there is one other (hacky) way to deal with this - use a udev script
to trigger update + reload of the system dnsmasq's configuration when
a USB NIC device hotplug/unplug occurs. That is clearly just crude 
patch over the already serious problem.

 My proposal is to get rid of the necessity for two dnsmasq instances. 
 Libvirt should check for the existance of a system dnsmasq and, if the 
 system daemon exists, libvirt should drop the required configuration 
 into /etc/dnsmasq.d and then restart it. If the 

Re: [libvirt] and for LXC?

2010-03-01 Thread Daniel Veillard
On Mon, Mar 01, 2010 at 04:14:34PM +0300, Mihamina Rakotomandimby wrote:
 Manao ahoana, Hello, Bonjour,
 
 Is there some plan/schedule to make libvirt LXC friendly?
 Or is it already?

  http://libvirt.org/

  livirt supports:
The LXC Linux container system 

http://libvirt.org/drvlxc.html

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] inability to open local read-only connection

2010-03-01 Thread Daniel P. Berrange
On Thu, Feb 25, 2010 at 02:34:02PM -0600, Tavares, John wrote:
 I have been experimenting with using libvirt (0.3.3) on a variety of
 systems (RHEL, CentOS and Oracle VM).  I have run into an issue when
 I try to open a local read-only connection to the hypervisor that is
 failing only on Oracle VM server release 2.2.0.  I have created a root
 owned setuid executable that is effectively running as root, but even 
 so, still cannot open the local read-only connection of the hypervisor.
  It only works if I run it directly as root.  This is not an option.  I
 do not understand why it works as is on my RHEL and CentOS machines, but
 not my Oracle machine.  It would seem as thought it is not checking if
 the effective uid is root, just the uid.

A readonly connection essentially just goes to a separate UNIX socket, which
has more relaxed permissions (mode 0777, instead of 0700). The kernel does
the permissions checking when attempting to open it, so it should be using
the effective ID. 

 Has anyone run into a similar issue or have any suggestions of what I 
 might try to fix this issue or can tell me that this is a defect that 
 needs (is) fixed??

I'd suggest trying to strace your process  see where it fails, and/or run
with LIBVIRT_DEBUG=1 environemnt variable set.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] Free the macvtap mode string

2010-03-01 Thread Daniel P. Berrange
On Fri, Feb 26, 2010 at 01:03:56PM -0500, Stefan Berger wrote:
 I forgot to free the macvtap mode string. This fixes it.
 
 Signed-off-by: Stefan Berger stef...@us.ibm.com
 

 Index: libvirt-macvtap/src/conf/domain_conf.c
 ===
 --- libvirt-macvtap.orig/src/conf/domain_conf.c
 +++ libvirt-macvtap/src/conf/domain_conf.c
 @@ -1974,6 +1974,7 @@ cleanup:
  VIR_FREE(type);
  VIR_FREE(internal);
  VIR_FREE(devaddr);
 +VIR_FREE(mode);
  
  return def;
  

ACK

Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] qemu: avoid null dereference on failed migration

2010-03-01 Thread Daniel P. Berrange
On Fri, Feb 26, 2010 at 10:17:53AM -0700, Eric Blake wrote:
 From: Eric Blake ebl...@redhat.com
 
 * src/qemu/qemu_monitor_text.c
 (qemuMonitorTextGetMigrationStatus): Check for failed strchr, to
 silence a coverity warning.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  src/qemu/qemu_monitor_text.c |5 +
  1 files changed, 5 insertions(+), 0 deletions(-)
 
 diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
 index 62ffcc6..e7b4b1f 100644
 --- a/src/qemu/qemu_monitor_text.c
 +++ b/src/qemu/qemu_monitor_text.c
 @@ -989,6 +989,11 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon,
  if ((tmp = strstr(reply, MIGRATION_PREFIX)) != NULL) {
  tmp += strlen(MIGRATION_PREFIX);
  end = strchr(tmp, '\r');
 +if (end == NULL) {
 +qemuReportError(VIR_ERR_INTERNAL_ERROR,
 +_(unexpected migration status in %s), reply);
 +goto cleanup;
 +}
  *end = '\0';
 
  if ((*status = qemuMonitorMigrationStatusTypeFromString(tmp))  0) {
 -- 

ACK


Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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/9] Introduce public API for domain async job handling

2010-03-01 Thread Daniel P. Berrange
On Fri, Feb 26, 2010 at 06:40:10PM +0100, Daniel Veillard wrote:
 On Thu, Feb 18, 2010 at 03:56:07PM +, Daniel P. Berrange wrote:
  Introduce a new public API that provides a way to get progress
  info on currently running jobs on a virDomainpPtr. APIs that
  are initially within scope of this idea are
  
   virDomainMigrate
   virDomainMigrateToURI
   virDomainSave
   virDomainRestore
   virDomainCoreDump
  
  These all take a potentially long time and benefit from monitoring.
  The virDomainJobInfo struct allows for various pieces of information
  to be reported
  
   - Percentage completion
   - Time
   - Overall data
   - Guest memory data
   - Guest disk/file data
  
  * include/libvirt/libvirt.h.in: Add virDomainGetJobInfo
  * python/generator.py, python/libvirt-override-api.xml,
python/libvirt-override.c: Override for virDomainGetJobInfo API
  * python/typewrappers.c, python/typewrappers.h: Introduce wrapper
for unsigned long long type
  ---
   include/libvirt/libvirt.h.in|   49 
  +++
   python/generator.py |1 +
   python/libvirt-override-api.xml |5 
   python/libvirt-override.c   |   36 
   python/typewrappers.c   |8 ++
   python/typewrappers.h   |1 +
   6 files changed, 100 insertions(+), 0 deletions(-)
  
  diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in
  index 260505e..51c0844 100644
  --- a/include/libvirt/libvirt.h.in
  +++ b/include/libvirt/libvirt.h.in
  @@ -1787,6 +1787,55 @@ char *virConnectBaselineCPU(virConnectPtr conn,
   unsigned int ncpus,
   unsigned int flags);
   
  +typedef enum {
  +VIR_DOMAIN_JOB_NONE  = 0, /* No job is active */
  +VIR_DOMAIN_JOB_BOUNDED   = 1, /* Job with a finite completion time */
  +VIR_DOMAIN_JOB_UNBOUNDED = 2, /* Job without a finite completion time 
  */
  +VIR_DOMAIN_JOB_COMPLETED = 3, /* Job has finished, but isn't cleaned 
  up */
  +VIR_DOMAIN_JOB_FAILED= 4, /* Job hit error, but isn't cleaned up */
 
 
   5 disapeared ?

Opps, bad counting !

 
  +VIR_DOMAIN_JOB_CANCELLED = 6, /* Job was aborted, but isn't cleaned up 
  */
  +} virDomainJobType;
  +
  +typedef struct _virDomainJobInfo virDomainJobInfo;
  +typedef virDomainJobInfo *virDomainJobInfoPtr;
  +struct _virDomainJobInfo {
  +/* One of virDomainJobType */
  +int type;
  +
  +/* Time is measured in seconds */
  +unsigned long long timeElapsed;/* Always set */
  +unsigned long long timeRemaining;  /* Only for VIR_DOMAIN_JOB_BOUNDED 
  */
  +
  +/* Data is measured in bytes unless otherwise specified
  + * and is measuring the job as a whole
  + *
  + * For VIR_DOMAIN_JOB_UNBOUNDED, dataTotal may be less
  + * than the final sum of dataProcessed + dataRemaining
  + * in the event that the hypervisor has to repeat some
  + * data eg due to dirtied pages during migration
  + *
  + * For VIR_DOMAIN_JOB_BOUNDED, dataTotal shall always
  + * equal sum of dataProcessed + dataRemaining
  + */
  +unsigned long long dataTotal;
  +unsigned long long dataProcessed;
  +unsigned long long dataRemaining;
  +
  +/* As above, but only tracking guest memory progress */
  +unsigned long long memTotal;
  +unsigned long long memProcessed;
  +unsigned long long memRemaining;
  +
  +/* As above, but only tracking guest disk file progress */
  +unsigned long long fileTotal;
  +unsigned long long fileProcessed;
  +unsigned long long fileRemaining;
  +};
 
   I wonder if we should not somehow provide an unit for the items being
 measured. Maybe we don't want bytes in the future

The idea here is that the 'mem' and 'file' fields are always in bytes.
The 'data' field units are defined by the type of job being executed,
though effectively they're always bytes too.

 
  +int virDomainGetJobInfo(virDomainPtr dom,
  +virDomainJobInfoPtr info);
  +
  +
   #ifdef __cplusplus
   }
   #endif
  diff --git a/python/generator.py b/python/generator.py
  index 24eaf50..f7625fd 100755
  --- a/python/generator.py
  +++ b/python/generator.py
  @@ -271,6 +271,7 @@ skip_impl = (
   'virConnGetLastError',
   'virGetLastError',
   'virDomainGetInfo',
  +'virDomainGetJobInfo',
   'virNodeGetInfo',
   'virDomainGetUUID',
   'virDomainGetUUIDString',
  diff --git a/python/libvirt-override-api.xml 
  b/python/libvirt-override-api.xml
  index 76a6fd5..1260c0c 100644
  --- a/python/libvirt-override-api.xml
  +++ b/python/libvirt-override-api.xml
  @@ -48,6 +48,11 @@
 return type='int *' info='the list of information or None in case 
  of error'/
 arg name='domain' type='virDomainPtr' info='a domain object'/
   /function
  +function name='virDomainGetJobInfo' file='python'
  +  infoExtract information about an 

Re: [libvirt] [PATCH 4/6] Fix PCI address handling when controllers are deleted.

2010-03-01 Thread Daniel P. Berrange
On Fri, Feb 26, 2010 at 02:09:17PM +0100, Wolfgang Mauerer wrote:
 When a controller is not present in the system anymore, the PCI address
 must be deleted from libvirt's hashtable because it can be re-used for
 other purposes.
 
 Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 ---
  src/qemu/qemu_driver.c |5 +
  1 files changed, 5 insertions(+), 0 deletions(-)
 
 diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
 index 5394ff5..8960ef8 100644
 --- a/src/qemu/qemu_driver.c
 +++ b/src/qemu/qemu_driver.c
 @@ -6280,6 +6280,11 @@ static int qemudDomainDetachPciControllerDevice(struct 
 qemud_driver *driver,
  VIR_FREE(vm-def-controllers);
  vm-def-ncontrollers = 0;
  }
 +
 +if (qemuDomainPCIAddressReleaseAddr(priv-pciaddrs, detach-info)  0) {
 +VIR_WARN0(Unable to release PCI address on controller);
 +}
 +
  virDomainControllerDefFree(detach);
  
  ret = 0;


ACK, I have a feeling we leak the address in other hotunplug code too


Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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/6] Always link with libgcrypt

2010-03-01 Thread Daniel P. Berrange
On Fri, Feb 26, 2010 at 02:09:15PM +0100, Wolfgang Mauerer wrote:
 ... not just when using the remote driver: libvirt.c always depends on it.
 This unbreaks builds with --without-remote
 
 Signed-off-by: Wolfgang Mauerer wolfgang.maue...@siemens.com
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 ---
  src/Makefile.am |7 ---
  1 files changed, 4 insertions(+), 3 deletions(-)

Hmm, I think we also need to make the TLS threads code in libvirt.c
conditional on WITH_REMOTE, since we don't want that compiled at
all if the remote driver isn't present

 
 diff --git a/src/Makefile.am b/src/Makefile.am
 index 2051e5d..8f0fd30 100644
 --- a/src/Makefile.am
 +++ b/src/Makefile.am
 @@ -334,8 +334,11 @@ libvirt_la_LIBADD += libvirt_driver.la
  libvirt_driver_la_SOURCES = $(DRIVER_SOURCES)
  
  libvirt_driver_la_CFLAGS = $(NUMACTL_CFLAGS) \
 + $(GNUTLS_CFLAGS)\
   -...@top_srcdir@/src/conf
 -libvirt_driver_la_LDFLAGS = $(NUMACTL_LIBS)
 +libvirt_driver_la_LDFLAGS = $(NUMACTL_LIBS) \
 + $(GNUTLS_LIBS)
 +
  
  USED_SYM_FILES = libvirt_private.syms
  
 @@ -362,11 +365,9 @@ noinst_LTLIBRARIES += libvirt_driver_remote.la
  libvirt_la_LIBADD += libvirt_driver_remote.la
  endif
  libvirt_driver_remote_la_CFLAGS =\
 - $(GNUTLS_CFLAGS)\
   $(SASL_CFLAGS)  \
   -...@top_srcdir@/src/conf
  libvirt_driver_remote_la_LDFLAGS =   \
 - $(GNUTLS_LIBS)  \
   $(SASL_LIBS)
  if WITH_DRIVER_MODULES
  libvirt_driver_remote_la_LDFLAGS += -module -avoid-version

Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] libvirt/dnsmasq integration.

2010-03-01 Thread Daniel P. Berrange
On Wed, Feb 24, 2010 at 04:37:04PM +, Simon Kelley wrote:
 Richard W.M. Jones wrote:
  On Wed, Feb 24, 2010 at 02:34:53PM +, Simon Kelley wrote:
  [...]
  Does that make sense? It's a long and involved explanation to come to  
  cold. I fear I may have over-simplified what libvirt is doing with  
  dnsmasq, in which case please enlighten me and I'll modify my scheme to  
  take that into account. If this looks good I can easily have the  
  necessary dnsmasq changes in the next release.
  
  I read through it and it makes sense to me.
  
  The only thing: is /etc/dnsmasq.d a true standard across all distros?
  It is for Fedora, but an old Debian distro I use does not have this
  directory.
 
 The ability to read config fragments from a directory has been in
 dnsmasq since version 2.32 (effectively, forever). The Debian (and by
 inheritance, Ubuntu) packages have created and automatically searched
 /etc/dnsmasq.d since 2.46, released in November 2008. That's not quite
 old enough to be in the current Debian stable, which has 2.45.
 
 All this is slighly moot, since this scheme needs changes to dnsmasq.
 Those can easily go into the next release.
 
  
  Is '/etc/init.d/dnsmasq restart' a reliable way to restart the system
  dnsmasq across all distros?  (Perhaps 'service dnsmasq restart' is
  better?).
 
 Almost certainly. Could this be a hook supplied by the distro packaging?

I'm not sure that the 'service' command is standardized on all distros
either, is it ? 

Does dnsmasq need a full restart, or is a SIGHUP sufficient ?

If the latter, then we could avoid the initscript entirely, by reading
dnsmasq's pid file, using kill(0, $PID) and check for -ERSCH to see if it
is running or not.  Then finally SIGHUP it to reload


Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] [macvtap] interface type direct web docs addition

2010-03-01 Thread Daniel P. Berrange
On Tue, Feb 23, 2010 at 12:31:07PM -0500, Stefan Berger wrote:
 This adds a description about the 'direct' type of interface recently
 added for macvtap device type support on the host.
 
 

 Index: libvirt-macvtap/docs/formatdomain.html.in
 ===
 --- libvirt-macvtap.orig/docs/formatdomain.html.in
 +++ libvirt-macvtap/docs/formatdomain.html.in
 @@ -674,6 +674,28 @@
.../pre
  
  
 +h5a name=elementsNICSDirectDirect attachment to physical 
 interface/a/h5
 +
 +p
 +  Provides direct attachment of the virtual machine's NIC to the given
 +  physial interface of the host. This setup requires the Linux macvtap
 +  driver to be available. One of the modes 'vepa', 'bridge' or 'private'
 +  can be chosen for the operation mode of the macvtap device, 'vepa'
 +  being the default mode.
 +/p
 +
 +pre
 +  ...
 +  lt;devicesgt;
 +lt;interface type='direct'/gt;
 +...
 +lt;interface type='direct'gt;
 +  lt;source dev='eth0' mode='vepa'/gt;
 +lt;/interfacegt;
 +  lt;/devicesgt;
 +  .../pre
 +
 +
  h5a name=elementsNICSEthernetGeneric ethernet connection/a/h5
  
  p

ACK, though it might be nice to describe the difference between the modes
'vepa', 'bridge' or 'private' for people who are not so familiar with this
support


Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] Do not search xenstore for disk device IDs

2010-03-01 Thread Daniel Veillard
On Tue, Feb 23, 2010 at 03:35:04PM -0700, Jim Fehlig wrote:
 Daniel P. Berrange wrote:
  On Mon, Feb 22, 2010 at 02:50:07PM -0700, Jim Fehlig wrote:

  Disk devices can be referenced by name in Xen, e.g. when modifying
  their configuration or remvoving them.  As such, don't search
  xenstore for a device ID corresponding to the disk device.  Instead,
  search the disks contained in the domain definition and use the
  disk's target name if found.
 
  This approach allows removing a disk when domain is inactive.  We
  obviously can't search xenstore when the domain is inactive.
  
 
  This sounds reasonable, but I'm wondering if old XenD support the
  lookup-based on name ?

 
 Yes, I had thought about this as well.  Guess it depends on how old we
 are talking about.  It exists in Xen 3.1.x and newer.  I wonder if we
 should even care about Xen 3.0.x.  The upstream tree hasn't been touched
 in 3 years.

  Well we still ship this, so it would be appreciated if it didn't
broke if libvirt got updated for some reasons ;-)

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] libvirt/dnsmasq integration.

2010-03-01 Thread Simon Kelley
Daniel P. Berrange wrote:
 On Wed, Feb 24, 2010 at 04:37:04PM +, Simon Kelley wrote:
 Richard W.M. Jones wrote:
 On Wed, Feb 24, 2010 at 02:34:53PM +, Simon Kelley wrote:
 [...]
 Does that make sense? It's a long and involved explanation to come to  
 cold. I fear I may have over-simplified what libvirt is doing with  
 dnsmasq, in which case please enlighten me and I'll modify my scheme to  
 take that into account. If this looks good I can easily have the  
 necessary dnsmasq changes in the next release.
 I read through it and it makes sense to me.

 The only thing: is /etc/dnsmasq.d a true standard across all distros?
 It is for Fedora, but an old Debian distro I use does not have this
 directory.
 The ability to read config fragments from a directory has been in
 dnsmasq since version 2.32 (effectively, forever). The Debian (and by
 inheritance, Ubuntu) packages have created and automatically searched
 /etc/dnsmasq.d since 2.46, released in November 2008. That's not quite
 old enough to be in the current Debian stable, which has 2.45.

 All this is slighly moot, since this scheme needs changes to dnsmasq.
 Those can easily go into the next release.

 Is '/etc/init.d/dnsmasq restart' a reliable way to restart the system
 dnsmasq across all distros?  (Perhaps 'service dnsmasq restart' is
 better?).
 Almost certainly. Could this be a hook supplied by the distro packaging?
 
 I'm not sure that the 'service' command is standardized on all distros
 either, is it ? 

I don't know. It exists on recent Debian and Ubuntu versions.
 
 Does dnsmasq need a full restart, or is a SIGHUP sufficient ?

A full restart: Once running it has dropped root, and some configuration
changes need root to establish. There's a subset of the configuration
that can be put in special files and reloaded by SIGHUP.
 
 If the latter, then we could avoid the initscript entirely, by reading
 dnsmasq's pid file, using kill(0, $PID) and check for -ERSCH to see if it
 is running or not.  Then finally SIGHUP it to reload
Simon.

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


Re: [libvirt] [PATCH 2/6] Add new clock mode allowing variable adjustments

2010-03-01 Thread Daniel Veillard
On Thu, Feb 18, 2010 at 05:54:28PM +, Daniel P. Berrange wrote:
 This introduces a third option for clock offset synchronization,
 that allows an arbitrary / variable adjustment to be set. In
 essence the XML contains the time delta in seconds, relative to
 UTC.
 
   clock offset='variable' adjustment='123465'/
 
 The difference from 'utc' mode, is that management apps should
 track adjustments and preserve them at next reboot.

  hum ... if the management layer start to track time of delta w.r.t.
UTC as defined by node clock, I also assume they manage the variations
in clock when migrating too (or that the nodes clocks are actually
properly sync'ed).

 * docs/schemas/domain.rng: Schema for new clock mode
 * src/conf/domain_conf.c, src/conf/domain_conf.h: Parse
   new clock time delta
 * src/libvirt_private.syms, src/util/xml.c, src/util/xml.h: Add
   virXPathLongLong() method
 ---
  docs/schemas/domain.rng  |   25 +---
  src/conf/domain_conf.c   |   18 +-
  src/conf/domain_conf.h   |5 
  src/libvirt_private.syms |1 +
  src/util/xml.c   |   54 
 ++
  src/util/xml.h   |5 +++-
  6 files changed, 101 insertions(+), 7 deletions(-)
 
 diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
 index 1ff0944..d295bfe 100644
 --- a/docs/schemas/domain.rng
 +++ b/docs/schemas/domain.rng
 @@ -297,12 +297,24 @@
define name=clock
  optional
element name=clock
 -attribute name=offset
 -  choice
 + choice
 +  attribute name=offset

  hum isn't that inserting tabs in the XML that we remove from time to
  time :-) ?

  valuelocaltime/value
 +   /attribute
 +  attribute name=offset
  valueutc/value
 -  /choice
 -/attribute
 +   /attribute
 +   group
 +attribute name=offset
 +  valuevariable/value
 + /attribute
 + optional
 +   attribute name=adjustment
 + ref name=timeDelta/
 +   /attribute
 + /optional
 +   /group
 + /choice
  empty/
/element
  /optional
 @@ -1567,4 +1579,9 @@
param name='pattern'[a-zA-Z0-9\-_]+/param
  /data
/define
 +  define name=timeDelta
 +data type=string

  actually we could use XSD integer there
http://www.w3.org/TR/xmlschema-2/#integer
but this won't change much

 +  param name=pattern(-|\+)?[0-9]+/param
 +/data
 +  /define
  /grammar
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index f86b4eb..49d5d19 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -231,7 +231,8 @@ VIR_ENUM_IMPL(virDomainNetdevMacvtap, 
 VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST,
  
  VIR_ENUM_IMPL(virDomainClockOffset, VIR_DOMAIN_CLOCK_OFFSET_LAST,
utc,
 -  localtime);
 +  localtime,
 +  variable);
  
  #define virDomainReportError(code, fmt...)   \
  virReportErrorHelper(NULL, VIR_FROM_DOMAIN, code, __FILE__,  \
 @@ -3492,6 +3493,13 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr 
 caps,
  } else {
  def-clock.offset = VIR_DOMAIN_CLOCK_OFFSET_UTC;
  }
 +switch (def-clock.offset) {
 +case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
 +if (virXPathLongLong(./clock/@adjustment, ctxt,
 + def-clock.adjustment)  0)
 +def-clock.adjustment = 0;

   Hum, should probably give an error message here 

 +break;
 +}
  
  def-os.bootloader = virXPathString(string(./bootloader), ctxt);
  def-os.bootloaderArgs = virXPathString(string(./bootloader_args), 
 ctxt);
 @@ -5399,8 +5407,14 @@ char *virDomainDefFormat(virDomainDefPtr def,
  if (virCPUDefFormatBuf(buf, def-cpu,   , 0)  0)
  goto cleanup;
  
 -virBufferVSprintf(buf,   clock offset='%s'/\n,
 +virBufferVSprintf(buf,   clock offset='%s',
virDomainClockOffsetTypeToString(def-clock.offset));
 +switch (def-clock.offset) {
 +case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE:
 +virBufferVSprintf(buf,  adjustment='%lld', def-clock.adjustment);
 +break;
 +}
 +virBufferAddLit(buf, /\n);
  
  if (virDomainLifecycleDefFormat(buf, def-onPoweroff,
  on_poweroff)  0)
 diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
 index fbbe683..f5fe016 100644
 --- a/src/conf/domain_conf.h
 +++ b/src/conf/domain_conf.h
 @@ -612,6 +612,7 @@ struct _virSecurityLabelDef {
  enum virDomainClockOffsetType {
  VIR_DOMAIN_CLOCK_OFFSET_UTC = 0,
  VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME = 1,
 +VIR_DOMAIN_CLOCK_OFFSET_VARIABLE = 2,
  
  VIR_DOMAIN_CLOCK_OFFSET_LAST,
  };
 @@ -620,6 +621,10 @@ typedef struct _virDomainClockDef virDomainClockDef;
  typedef virDomainClockDef *virDomainClockDefPtr;
  struct _virDomainClockDef {
  int offset;
 +
 +

Re: [libvirt] [PATCH 3/6] Support variable clock offset mode in QEMU

2010-03-01 Thread Daniel Veillard
On Thu, Feb 18, 2010 at 05:54:29PM +, Daniel P. Berrange wrote:
 This allows QEMU guests to be started with an arbitrary clock
 offset
 
 The test case can't actually be enabled, since QEMU argv expects
 an absolute timestring, and this will obviously change every
 time the test runs :-( Hopefully QEMU will allow a relative
 time offset in the future.
 
 * src/qemu/qemu_conf.c, src/qemu/qemu_conf.h: Use the -rtc arg
   if available to support variable clock offset mode
 * tests/qemuhelptest.c: Add QEMUD_CMD_FLAG_RTC for qemu 0.12.1
 * qemuxml2argvdata/qemuxml2argv-clock-variable.args,
   qemuxml2argvdata/qemuxml2argv-clock-variable.xml,
   qemuxml2argvtest.c: Test case, except we can't actually enable
   it yet.
 ---
  src/qemu/qemu_conf.c   |   82 
 ++--
  src/qemu/qemu_conf.h   |1 +
  tests/qemuhelptest.c   |3 +-
  .../qemuxml2argv-clock-variable.args   |1 +
  .../qemuxml2argv-clock-variable.xml|   24 ++
  tests/qemuxml2argvtest.c   |4 +
  6 files changed, 107 insertions(+), 8 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-variable.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-variable.xml
 
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index a207fc7..112a7c2 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -1155,6 +1155,9 @@ static unsigned long long qemudComputeCmdFlags(const 
 char *help,
  flags |= QEMUD_CMD_FLAG_BALLOON;
  if (strstr(help, -device))
  flags |= QEMUD_CMD_FLAG_DEVICE;
 +/* The trailing ' ' is important to avoid a bogus match */
 +if (strstr(help, -rtc ))
 +flags |= QEMUD_CMD_FLAG_RTC;
  /* Keep disabled till we're actually ready to turn on netdev mode
   * The plan is todo it in 0.13.0 QEMU, but lets wait  see... */
  #if 0
 @@ -2969,6 +2972,56 @@ error:
  }
  
  
 +static char *
 +qemuBuildClockArgStr(virDomainClockDefPtr def)
 +{
 +virBuffer buf = VIR_BUFFER_INITIALIZER;
 +
 +switch (def-offset) {
 +case VIR_DOMAIN_CLOCK_OFFSET_UTC:
 +virBufferAddLit(buf, base=utc);
 +break;
 +
 +case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
 +virBufferAddLit(buf, base=localtime);
 +break;
 +
 +case VIR_DOMAIN_CLOCK_OFFSET_VARIABLE: {
 +time_t now = time(NULL);
 +struct tm nowbits;
 +
 +now += def-adjustment;
 +gmtime_r(now, nowbits);
 +
 +virBufferVSprintf(buf, base=%d-%d-%dT%d:%d:%d,
 +  nowbits.tm_year,
 +  nowbits.tm_mon,
 +  nowbits.tm_mday,
 +  nowbits.tm_hour,
 +  nowbits.tm_min,
 +  nowbits.tm_sec);
 +}   break;
 +
 +default:
 +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +_(unsupported clock offset '%s'),
 +virDomainClockOffsetTypeToString(def-offset));
 +goto error;
 +}
 +
 +if (virBufferError(buf)) {
 +virReportOOMError();
 +goto error;
 +}
 +
 +return virBufferContentAndReset(buf);
 +
 +error:
 +virBufferFreeAndReset(buf);
 +return NULL;
 +}
 +
 +
  static int
  qemuBuildCpuArgStr(const struct qemud_driver *driver,
 const virDomainDefPtr def,
 @@ -3400,13 +3453,28 @@ int qemudBuildCommandLine(virConnectPtr conn,
  }
  }
  
 -if (def-clock.offset == VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME)
 -ADD_ARG_LIT(-localtime);
 -else if (def-clock.offset != VIR_DOMAIN_CLOCK_OFFSET_UTC) {
 -qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 -_(unsupported clock offset '%s'),
 -virDomainClockOffsetTypeToString(def-clock.offset));
 -goto error;
 +if (qemuCmdFlags  QEMUD_CMD_FLAG_RTC) {
 +const char *rtcopt;
 +ADD_ARG_LIT(-rtc);
 +if (!(rtcopt = qemuBuildClockArgStr(def-clock)))
 +goto error;
 +ADD_ARG(rtcopt);
 +} else {
 +switch (def-clock.offset) {
 +case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
 +ADD_ARG_LIT(-localtime);
 +break;
 +
 +case VIR_DOMAIN_CLOCK_OFFSET_UTC:
 +/* Nothing, its the default */
 +break;
 +
 +default:
 +qemuReportError(VIR_ERR_CONFIG_UNSUPPORTED,
 +_(unsupported clock offset '%s'),
 +
 virDomainClockOffsetTypeToString(def-clock.offset));
 +goto error;
 +}
  }
  
  if ((qemuCmdFlags  QEMUD_CMD_FLAG_NO_REBOOT) 
 diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
 index 7041489..e32a3d7 100644
 --- a/src/qemu/qemu_conf.h
 +++ b/src/qemu/qemu_conf.h
 @@ -82,6 +82,7 @@ enum qemud_cmd_flags {
  QEMUD_CMD_FLAG_SDL   = 

Re: [libvirt] [PATCH 5/6] Allow configurable timezones with QEMU

2010-03-01 Thread Daniel Veillard
On Thu, Feb 18, 2010 at 05:54:31PM +, Daniel P. Berrange wrote:
 Allow an arbitrary timezone with QEMU by setting the $TZ environment
 variable when launching QEMU
 
 * src/qemu/qemu_conf.c: Set TZ environment variable if a timezone
   is requested
 * tests/qemuxml2argvtest.c: Add test case for timezones
 * tests/qemuxml2argvdata/qemuxml2argv-clock-france.xml,
   tests/qemuxml2argvdata/qemuxml2argv-clock-france.args: Data
   for timezone tests
 ---
  src/qemu/qemu_conf.c   |6 +
  .../qemuxml2argv-clock-france.args |1 +
  .../qemuxml2argvdata/qemuxml2argv-clock-france.xml |   24 
 
  tests/qemuxml2argvtest.c   |2 +
  4 files changed, 33 insertions(+), 0 deletions(-)
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-france.args
  create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-clock-france.xml
 
 diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
 index cd0dd7f..4ed00cb 100644
 --- a/src/qemu/qemu_conf.c
 +++ b/src/qemu/qemu_conf.c
 @@ -2983,6 +2983,7 @@ qemuBuildClockArgStr(virDomainClockDefPtr def)
  break;
  
  case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
 +case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE:
  virBufferAddLit(buf, base=localtime);
  break;
  
 @@ -3462,6 +3463,7 @@ int qemudBuildCommandLine(virConnectPtr conn,
  } else {
  switch (def-clock.offset) {
  case VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME:
 +case VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE:
  ADD_ARG_LIT(-localtime);
  break;
  
 @@ -3476,6 +3478,10 @@ int qemudBuildCommandLine(virConnectPtr conn,
  goto error;
  }
  }
 +if (def-clock.offset == VIR_DOMAIN_CLOCK_OFFSET_TIMEZONE 
 +def-clock.data.timezone) {
 +ADD_ENV_PAIR(TZ, def-clock.data.timezone);
 +}
  
  if ((qemuCmdFlags  QEMUD_CMD_FLAG_NO_REBOOT) 
  def-onReboot != VIR_DOMAIN_LIFECYCLE_RESTART)
 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args 
 b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args
 new file mode 100644
 index 000..3c57397
 --- /dev/null
 +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.args
 @@ -0,0 +1 @@
 +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test TZ=Europe/Paris 
 /usr/bin/qemu -S -M pc -m 214 -smp 1 -nographic -monitor 
 unix:/tmp/test-monitor,server,nowait -rtc base=localtime -no-acpi -boot c 
 -hda /dev/HostVG/QEMUGuest1 -net none -serial none -parallel none -usb
 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-clock-france.xml 
 b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.xml
 new file mode 100644
 index 000..157fdfb
 --- /dev/null
 +++ b/tests/qemuxml2argvdata/qemuxml2argv-clock-france.xml
 @@ -0,0 +1,24 @@
 +domain type='qemu'
 +  nameQEMUGuest1/name
 +  uuidc7a5fdbd-edaf-9455-926a-d65c16db1809/uuid
 +  memory219200/memory
 +  currentMemory219200/currentMemory
 +  vcpu1/vcpu
 +  os
 +type arch='i686' machine='pc'hvm/type
 +boot dev='hd'/
 +  /os
 +  clock offset='timezone' timezone='Europe/Paris'/
 +  on_poweroffdestroy/on_poweroff
 +  on_rebootrestart/on_reboot
 +  on_crashdestroy/on_crash
 +  devices
 +emulator/usr/bin/qemu/emulator
 +disk type='block' device='disk'
 +  source dev='/dev/HostVG/QEMUGuest1'/
 +  target dev='hda' bus='ide'/
 +  address type='drive' controller='0' bus='0' unit='0'/
 +/disk
 +controller type='ide' index='0'/
 +  /devices
 +/domain
 diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
 index 510176c..bd41b6c 100644
 --- a/tests/qemuxml2argvtest.c
 +++ b/tests/qemuxml2argvtest.c
 @@ -230,6 +230,8 @@ mymain(int argc, char **argv)
   * Can't be enabled since the absolute timestamp changes every time
  DO_TEST(clock-variable, QEMUD_CMD_FLAG_RTC);
  */
 +DO_TEST(clock-france, QEMUD_CMD_FLAG_RTC);
 +
  DO_TEST(hugepages, QEMUD_CMD_FLAG_MEM_PATH);
  DO_TEST(disk-cdrom, 0);
  DO_TEST(disk-cdrom-empty, QEMUD_CMD_FLAG_DRIVE);

  ACK,

Daniel

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

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


Re: [libvirt] [PATCH] Do not search xenstore for disk device IDs

2010-03-01 Thread Daniel P. Berrange
On Mon, Mar 01, 2010 at 02:45:41PM +0100, Daniel Veillard wrote:
 On Tue, Feb 23, 2010 at 03:35:04PM -0700, Jim Fehlig wrote:
  Daniel P. Berrange wrote:
   On Mon, Feb 22, 2010 at 02:50:07PM -0700, Jim Fehlig wrote:
 
   Disk devices can be referenced by name in Xen, e.g. when modifying
   their configuration or remvoving them.  As such, don't search
   xenstore for a device ID corresponding to the disk device.  Instead,
   search the disks contained in the domain definition and use the
   disk's target name if found.
  
   This approach allows removing a disk when domain is inactive.  We
   obviously can't search xenstore when the domain is inactive.
   
  
   This sounds reasonable, but I'm wondering if old XenD support the
   lookup-based on name ?
 
  
  Yes, I had thought about this as well.  Guess it depends on how old we
  are talking about.  It exists in Xen 3.1.x and newer.  I wonder if we
  should even care about Xen 3.0.x.  The upstream tree hasn't been touched
  in 3 years.
 
   Well we still ship this, so it would be appreciated if it didn't
 broke if libvirt got updated for some reasons ;-)

Yep, RHEL-5  still ships with Xen 3.0.x series  we aim to support that
in libvirt. I think we can probably just make your new approach be
conditional on the xendConfigVersion variable we already have.


Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] qemu: avoid null dereference on failed migration

2010-03-01 Thread Daniel Veillard
On Fri, Feb 26, 2010 at 10:17:53AM -0700, Eric Blake wrote:
 From: Eric Blake ebl...@redhat.com
 
 * src/qemu/qemu_monitor_text.c
 (qemuMonitorTextGetMigrationStatus): Check for failed strchr, to
 silence a coverity warning.
 
 Signed-off-by: Eric Blake ebl...@redhat.com
 ---
  src/qemu/qemu_monitor_text.c |5 +
  1 files changed, 5 insertions(+), 0 deletions(-)
 
 diff --git a/src/qemu/qemu_monitor_text.c b/src/qemu/qemu_monitor_text.c
 index 62ffcc6..e7b4b1f 100644
 --- a/src/qemu/qemu_monitor_text.c
 +++ b/src/qemu/qemu_monitor_text.c
 @@ -989,6 +989,11 @@ int qemuMonitorTextGetMigrationStatus(qemuMonitorPtr mon,
  if ((tmp = strstr(reply, MIGRATION_PREFIX)) != NULL) {
  tmp += strlen(MIGRATION_PREFIX);
  end = strchr(tmp, '\r');
 +if (end == NULL) {
 +qemuReportError(VIR_ERR_INTERNAL_ERROR,
 +_(unexpected migration status in %s), reply);
 +goto cleanup;
 +}
  *end = '\0';
 
  if ((*status = qemuMonitorMigrationStatusTypeFromString(tmp))  0) {

  ACK, 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


Re: [libvirt] [PATCH 6/6] Implement hotremove for SCSI disks

2010-03-01 Thread Gerd Hoffmann

On 03/01/10 15:29, Wolfgang Mauerer wrote:

Gerd: Are you intending to add the drive_del feature, or is
the approach outlined above sufficient for drive hotplug/remove?


I'm busy with other tasks right now.


Respectively can there be any problems if we remove a device
associated with a disk and then re-create a drive/device pair
with the same IDs as before, but with a different configuration?


There are no problems with that, when it is gone from 'info block' all 
traces of the drive are gone and creating another one with the same name 
is not a problem.


For symmetry reasons it would be nice to have a drive_del command though 
and have device_del not implicitly zap the drive.  The current scheme 
also fails to handle some corner cases like device_add failing (you are 
left with a drive you can't remove easily).


cheers,
  Gerd

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


[libvirt] [PATCH] cmdPoolDiscoverSources: initialize earlier to avoid FP from clang

2010-03-01 Thread Jim Meyering
This change hoists the initialization of srcSpec to the top,
to keep clang from reporting a false-positive used-uninitialized problem.


From f487655124c887310be3bc82d7beae3cd47e7d21 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 1 Mar 2010 15:41:15 +0100
Subject: [PATCH] cmdPoolDiscoverSources: initialize earlier to avoid FP from 
clang

* tools/virsh.c (cmdPoolDiscoverSources): Always initialize srcSpec.
Otherwise, clang would report that srcSpec could be used uninitialized
in the call to virConnectFindStoragePoolSources.
---
 tools/virsh.c |7 +++
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/virsh.c b/tools/virsh.c
index 3b1011c..191e7ba 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -4431,17 +4431,16 @@ static const vshCmdOptDef 
opts_find_storage_pool_sources[] = {
 static int
 cmdPoolDiscoverSources(vshControl * ctl, const vshCmd * cmd ATTRIBUTE_UNUSED)
 {
-char *type, *srcSpec, *srcSpecFile, *srcList;
+char *type, *srcSpecFile, *srcList;
+char *srcSpec = NULL;
 int found;

 type = vshCommandOptString(cmd, type, found);
 if (!found)
 return FALSE;
 srcSpecFile = vshCommandOptString(cmd, srcSpec, found);
-if (!found) {
+if (!found)
 srcSpecFile = NULL;
-srcSpec = NULL;
-}

 if (!vshConnectionUsability(ctl, ctl-conn, TRUE))
 return FALSE;
--
1.7.0.1.414.g89213d

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


Re: [libvirt] libvirt/dnsmasq integration.

2010-03-01 Thread Daniel P. Berrange
On Mon, Mar 01, 2010 at 01:54:44PM +, Simon Kelley wrote:
 Daniel P. Berrange wrote:
 
  I imagine you've already seen this, but as an example of the ARGV that
  libvirt generates for its dnsmasq instances:
  
/usr/sbin/dnsmasq \
 --strict-order \
 --bind-interfaces \
 --pid-file=/var/run/libvirt/network/default.pid \
 --conf-file=  \
 --listen-address 192.168.122.1 \
 --except-interface lo \
 --dhcp-range 192.168.122.2,192.168.122.254 \
 --dhcp-lease-max=253
  
  
 
 That's useful to know. For new releases, --dhcp-lease-max is no longer
 needed,(the default changed from 150 to 1000). Why are you using
 --strict-order? That's not normally a sensible choice.

We had bug reports from users that VMs weren't doing DNS resolution in
the same way as apps on the host. We determined that GLibc's resolver
was applying strict ordering, thus enabling --strict-order in dnsmasq
gives us consistent behaviour.

  My other concern with writing libvirt configs into /etc/dnsmasq.d is that
  users will then get the impression that this is something that they can
  freely edit / modify at will. They'll be unhappy with libvirt overwrites
  their changes whenever it starts.  This could perhaps be addressed by 
  allowing use to put the configs into /var/lib/dnsmasq/ instead of 
  /etc/dnsmasq.d, which is more common location for non-user editable 
  configs generated at runtime. 
 
 That's a packaging, rather then dnsmasq, issue: /etc/dnsmasq.d is not
 hard-coded into the binary, it's supplied on the command line by the
 start-up script. (again this is for Debian and Ubuntu). There's no
 reason why /var/lib/dnsmasq/ couldn't be added too. The downside of that
 would be more that it's more mysterious for users trying to work out
 exactly where all this configuration is coming from. Maybe a good
 compromise might be a DO NOT EDIT header in /etc/dnsmasq.d/libvirt.

Ok, that's good to know

  Your general plan of having a single dnsmasq instance though does sound
  desirable, given the way the sockets() APIs work wrt binding to addresses
  
  The total set of DNSMASQ args that we currently use are
  
--strict-order
--bind-interfaces
--domain DOMAIN-NAME(optional)
--pid-file=/var/run/libvirt/network/$NETWORK.pid
--conf-file=
--listen-address=IPADDR-OF-BRIDGE
--except-interface=lo
--dhcp-range=IPRANGE(optional, multiple times)
--dhcp-lease-max=RANGE-SIZE (optional)
--dhcp-host=STATIC-HOST-MAPPING (optional)
--enable-tftp   (optional)
--tftp-root=/some/path  (optional)
--dhcp-boot=PXE-BOOT-SERVER (optional)
 
 OK, the use of TFTP by libvirt is new to me. Looks like I'll have to
 pull some similar tricks to allow libvirt to enable TFTP on on interface
 without disturbing things elsewhere.

We only very recently added the TFTP/boot options so it is probably
not visible in many Linux distros yet.

   
  NB, we explicitly give a NULL conf-file in order to prevent any of the
  user's settings from the system instance from conflicting with libvirts
  settings. We don't really want users to be able to specify arbitrary
  other configuration settings for the libvirt dnsmasq instances, other
  than those we enable via the libvirt XML configuration.
  
  I've used 'optional' to denote flags we only pass when explicitly 
  configured via libvirt's XML format. The others we pass all the
  time.
  
  The 'lease-max' arg we calculate to be exactly matching the number of
  addresses in the configured dhcp-range args. This is because some of
  our users had configured dhcp ranges larger than 150 addresses in len.
 
 See comment earlier - the default has gone up to 1000, so you may not
 need this now.

Yep, that sounds pretty reasonable.

  Would this scheme allow libvirt to guarantee that no DHCP is present
  on its interface ? We currently support running in DNS-only mode, or
  DNS+DHCP. It is desirable to keep that regardless of how the host's
  system dnsmasq is currently configured for other interfaces. 
  
  If I am understanding your suggestion, this allows libvirt to easily
  enable DNS+DHCP mode on its own interface, without us accidentally 
  enabling DHCP on other host interfaces. 
  
  If libvirt doesn't use any --dhcp-range flags, there is still a chance
  that DHCP could be enabled on libvirt's interface  if the system dnsmasq
  had any dhcp-range args. Though assuming the IP ranges don't overlap
  this should be effectively a no-op ?
 
 
 Just emit
 
 --no-dhcp-interface=virb0
 
 instead of a dhcp-range. That solves that problem.

Ok

  I think you've got the general picture of what we're doing with dnsmasq. 
  
  At a very high level our original goals were
  
   - Support multiple independantly configured networks (virbr0, virbr1, etc)
   - Isolation between libvirt network interface config  host inteface config
   - Only support configuratin of options via 

Re: [libvirt] [PATCH 6/6] Implement hotremove for SCSI disks

2010-03-01 Thread Daniel P. Berrange
On Mon, Mar 01, 2010 at 03:40:14PM +0100, Gerd Hoffmann wrote:
 On 03/01/10 15:29, Wolfgang Mauerer wrote:
 Gerd: Are you intending to add the drive_del feature, or is
 the approach outlined above sufficient for drive hotplug/remove?
 
 I'm busy with other tasks right now.
 
 Respectively can there be any problems if we remove a device
 associated with a disk and then re-create a drive/device pair
 with the same IDs as before, but with a different configuration?
 
 There are no problems with that, when it is gone from 'info block' all 
 traces of the drive are gone and creating another one with the same name 
 is not a problem.

Great, this is sufficient for the hot-unplug code to work.

 For symmetry reasons it would be nice to have a drive_del command though 
 and have device_del not implicitly zap the drive.  The current scheme 
 also fails to handle some corner cases like device_add failing (you are 
 left with a drive you can't remove easily).

Yep, that's a minor edge case we can worry about later


Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] [RFC PATCH] build: consistently use C99 varargs macros

2010-03-01 Thread Eric Blake
Prior to this patch, there was an inconsistent mix between GNU and
C99.  For consistency, and potential portability to other compilers,
stick with the C99 vararg macro syntax.

* src/conf/cpu_conf.c (virCPUReportError): Use C99 rather than GNU
vararg macro syntax.
* src/conf/domain_conf.c (virDomainReportError): Likewise.
* src/conf/domain_event.c (eventReportError): Likewise.
* src/conf/interface_conf.c (virInterfaceReportError): Likewise.
* src/conf/network_conf.c (virNetworkReportError): Likewise.
* src/conf/node_device_conf.h (virNodeDeviceReportError):
Likewise.
* src/conf/secret_conf.h (virSecretReportError): Likewise.
* src/conf/storage_conf.h (virStorageReportError): Likewise.
---

Any objections to expanding this into a full-blown patch series?  This
touches 8 files; I counted 43 more files that could be altered, using:
$ git grep -l 'define.*[a-zA-Z]\.\.\.'

 src/conf/cpu_conf.c |4 ++--
 src/conf/domain_conf.c  |4 ++--
 src/conf/domain_event.c |5 +++--
 src/conf/interface_conf.c   |4 ++--
 src/conf/network_conf.c |6 +++---
 src/conf/node_device_conf.h |5 +++--
 src/conf/secret_conf.h  |6 +++---
 src/conf/storage_conf.h |6 +++---
 8 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index ed83188..612e376 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -31,9 +31,9 @@

 #define VIR_FROM_THIS VIR_FROM_CPU

-#define virCPUReportError(code, fmt...)   \
+#define virCPUReportError(code, ...)  \
 virReportErrorHelper(NULL, VIR_FROM_CPU, code, __FILE__,  \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 VIR_ENUM_IMPL(virCPUMatch, VIR_CPU_MATCH_LAST,
   minimum,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 4420445..ea84863 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -232,9 +232,9 @@ VIR_ENUM_IMPL(virDomainNetdevMacvtap, 
VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST,
   private,
   bridge)

-#define virDomainReportError(code, fmt...)   \
+#define virDomainReportError(code, ...)  \
 virReportErrorHelper(NULL, VIR_FROM_DOMAIN, code, __FILE__,  \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 #ifndef PROXY

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index b474b1c..e791cab 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1,6 +1,7 @@
 /*
  * domain_event.c: domain event queue processing helpers
  *
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2008 VirtualIron
  *
  * This library is free software; you can redistribute it and/or
@@ -30,9 +31,9 @@

 #define VIR_FROM_THIS VIR_FROM_NONE

-#define eventReportError(conn, code, fmt...)\
+#define eventReportError(conn, code, ...)   \
 virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__,   \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)


 /**
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index a0d2dfa..bc9e958 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -45,9 +45,9 @@ static int
 virInterfaceDefDevFormat(virBufferPtr buf,
  const virInterfaceDefPtr def, int level);

-#define virInterfaceReportError(code, fmt...)   \
+#define virInterfaceReportError(code, ...)  \
 virReportErrorHelper(NULL, VIR_FROM_INTERFACE, code, __FILE__,  \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 static
 void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 6d3c3c0..39ebd62 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1,7 +1,7 @@
 /*
  * network_conf.c: network XML handling
  *
- * Copyright (C) 2006-2009 Red Hat, Inc.
+ * Copyright (C) 2006-2010 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -53,9 +53,9 @@ VIR_ENUM_IMPL(virNetworkForward,
   VIR_NETWORK_FORWARD_LAST,
   none, nat, route )

-#define virNetworkReportError(code, fmt...) \
+#define virNetworkReportError(code, ...)\
 virReportErrorHelper(NULL, VIR_FROM_NETWORK, code, __FILE__,\
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets,
   const 

Re: [libvirt] [PATCH 0/3] Refactor XML parsing code

2010-03-01 Thread Daniel Veillard
On Thu, Feb 25, 2010 at 01:12:47PM +0100, Daniel Veillard wrote:
 On Wed, Feb 24, 2010 at 11:07:31PM +0100, Jiri Denemark wrote:
  Almost identical XML parsing code for either strings or files was copied
  15 times. Since it was needed on two other places for parsing CPU XML
  strings with correct error reporting, I decided to factor the parsing code
  out to xml.c and used this one implementation in all other places.
 
  looking very quickly, this looks sensible, but I can't fully review
  this right now, ACK on principle though, maybe this can wait for 0.7.8.

  Okay, reread them, looks just fine by me, ACK, let's push them
once 0.7.7 is out :-)

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] qemudNetworkIfaceConnect: remove dead store

2010-03-01 Thread Eric Blake
According to Jim Meyering on 3/1/2010 8:06 AM:
 clang reported this dead store.
 @@ -1520,11 +1520,10 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
  errobj = virSaveLastError();
  virNetworkFree(network);
  virSetError(errobj);
  virFreeError(errobj);
 
 -errobj = virSaveLastError();

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] udevEnumerateDevices: remove dead code

2010-03-01 Thread Dave Allan

On 03/01/2010 10:15 AM, Jim Meyering wrote:

Clang reported the dead store.



From 0557e23ca2f5eb78ed9f260040895011785f670f Mon Sep 17 00:00:00 2001

From: Jim Meyeringmeyer...@redhat.com
Date: Mon, 1 Mar 2010 16:14:35 +0100
Subject: [PATCH] udevEnumerateDevices: remove dead code

* src/node_device/node_device_udev.c (udevEnumerateDevices): Remove
unnecessary call to udev_list_entry_get_name.
---
  src/node_device/node_device_udev.c |4 +---
  1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/src/node_device/node_device_udev.c 
b/src/node_device/node_device_udev.c
index 68509f4..eee44c4 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -1,11 +1,11 @@
  /*
   * node_device_udev.c: node device enumeration - libudev implementation
   *
- * Copyright (C) 2009 Red Hat
+ * Copyright (C) 2009-2010 Red Hat
   *
   * This library is free software; you can redistribute it and/or
   * modify it under the terms of the GNU Lesser General Public
   * License as published by the Free Software Foundation; either
   * version 2.1 of the License, or (at your option) any later version.
   *
   * This library is distributed in the hope that it will be useful,
@@ -1333,30 +1333,28 @@ static int udevProcessDeviceListEntry(struct udev *udev,
  }


  static int udevEnumerateDevices(struct udev *udev)
  {
  struct udev_enumerate *udev_enumerate = NULL;
  struct udev_list_entry *list_entry = NULL;
-const char *name = NULL;
  int ret = 0;

  udev_enumerate = udev_enumerate_new(udev);

  ret = udev_enumerate_scan_devices(udev_enumerate);
  if (0 != ret) {
  VIR_ERROR(udev scan devices returned %d, ret);
  goto out;
  }

  udev_list_entry_foreach(list_entry,
  udev_enumerate_get_list_entry(udev_enumerate)) {

  udevProcessDeviceListEntry(udev, list_entry);
-name = udev_list_entry_get_name(list_entry);
  }

  out:
  udev_enumerate_unref(udev_enumerate);
  return ret;
  }

--
1.7.0.1.414.g89213d

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


ACK

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


Re: [libvirt] [PATCH] qemudNetworkIfaceConnect: remove dead store

2010-03-01 Thread Jim Meyering
Eric Blake wrote:
 According to Jim Meyering on 3/1/2010 8:06 AM:
 clang reported this dead store.
 @@ -1520,11 +1520,10 @@ qemudNetworkIfaceConnect(virConnectPtr conn,
  errobj = virSaveLastError();
  virNetworkFree(network);
  virSetError(errobj);
  virFreeError(errobj);

 -errobj = virSaveLastError();

 ACK.

Thanks for the quick reviews.
I'll push as soon as make-distcheck-after-rebase passes.

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


[libvirt] [PATCHv2] Make domain save work on root-squash NFS

2010-03-01 Thread Laine Stump
Move *all* file operations related to creation and writing of libvirt
header to the domain save file into a hook function that is called by
virFileOperation. First try to call virFileOperation as root. If that
fails with EACCESS, and (in the case of Linux) statfs says that we're
trying to save the file on an NFS share, rerun virFileOperation,
telling it to fork a child process and setuid to the qemu user. This
is the only way we can successfully create a file on a root-squashed
NFS server.

This patch (along with setting dynamic_ownership=0 in qemu.conf)
makes qemudDomainSave work on root-squashed NFS.
---

Note that this version of the patch avoids a problem the original had
with NFS shares whose directories aren't even readable by root - in
those cases, statfs would fail, so we would never learn that the file
was on NFS, and just fail the entire operation. The solution is to
repeatedly call statfs on a smaller and smaller part of the full path
until it succeeds - this will at most continue until the mount point
of the share, which will properly report the fstype for the share,
rather than for the mount point itself.

 src/qemu/qemu_driver.c |  165 
 1 files changed, 139 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 1e4b493..da92cf3 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -47,6 +47,11 @@
 #include sys/ioctl.h
 #include sys/un.h
 
+#ifdef __linux__
+#include sys/vfs.h
+#include linux/magic.h
+#endif
+
 #include virterror_internal.h
 #include logging.h
 #include datatypes.h
@@ -3956,14 +3961,44 @@ struct qemud_save_header {
 int unused[15];
 };
 
+struct fileOpHookData {
+virDomainPtr dom;
+const char *path;
+char *xml;
+struct qemud_save_header *header;
+};
+
+static int qemudDomainSaveFileOpHook(int fd, void *data) {
+struct fileOpHookData *hdata = data;
+int ret = 0;
+
+if (safewrite(fd, hdata-header, sizeof(*hdata-header)) != 
sizeof(*hdata-header)) {
+ret = errno;
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _(failed to write save header to '%s'), 
hdata-path);
+goto endjob;
+}
+
+if (safewrite(fd, hdata-xml, hdata-header-xml_len) != 
hdata-header-xml_len) {
+ret = errno;
+qemuReportError(VIR_ERR_OPERATION_FAILED,
+ _(failed to write xml to '%s'), hdata-path);
+goto endjob;
+}
+endjob:
+return ret;
+}
+
+
 static int qemudDomainSave(virDomainPtr dom,
const char *path)
 {
 struct qemud_driver *driver = dom-conn-privateData;
 virDomainObjPtr vm = NULL;
-int fd = -1;
 char *xml = NULL;
 struct qemud_save_header header;
+struct fileOpHookData hdata;
+int bypassSecurityDriver = 0;
 int ret = -1;
 int rc;
 virDomainEventPtr event = NULL;
@@ -4027,34 +4062,113 @@ static int qemudDomainSave(virDomainPtr dom,
 }
 header.xml_len = strlen(xml) + 1;
 
+/* Setup hook data needed by virFileOperation hook function */
+hdata.dom = dom;
+hdata.path = path;
+hdata.xml = xml;
+hdata.header = header;
+
 /* Write header to file, followed by XML */
-if ((fd = open(path, O_CREAT|O_TRUNC|O_WRONLY, S_IRUSR|S_IWUSR))  0) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-_(failed to create '%s'), path);
-goto endjob;
-}
 
-if (safewrite(fd, header, sizeof(header)) != sizeof(header)) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
-%s, _(failed to write save header));
-goto endjob;
-}
+/* First try creating the file as root */
+if ((rc = virFileOperation(path, O_CREAT|O_TRUNC|O_WRONLY,
+   S_IRUSR|S_IWUSR,
+   getuid(), getgid(),
+   qemudDomainSaveFileOpHook, hdata,
+   0)) != 0) {
+
+/* If we failed as root, and the error was permission-denied
+   (EACCES), assume it's on a network-connected share where
+   root access is restricted (eg, root-squashed NFS). If the
+   qemu user (driver-user) is non-root, just set a flag to
+   bypass security driver shenanigans, and retry the operation
+   after doing setuid to qemu user */
+
+if ((rc != EACCES) ||
+driver-user == getuid()) {
+virReportSystemError(rc, _(Failed to create domain save file 
'%s'),
+ path);
+goto endjob;
+}
 
-if (safewrite(fd, xml, header.xml_len) != header.xml_len) {
-qemuReportError(VIR_ERR_OPERATION_FAILED,
- %s, _(failed to write xml));
-goto endjob;
-}
+#ifdef __linux__
+/* On Linux we can also verify the FS-type of the directory. */
+char *dirpath, *p;
+struct statfs st;
+int 

Re: [libvirt] [RFC] Proposal for introduction of network traffic filtering capabilities for filtering of network traffic from and to VMs

2010-03-01 Thread Daniel P. Berrange
On Mon, Feb 22, 2010 at 01:45:20PM +0100, Gerhard Stenzel wrote:
 
 Hi, here is a preview of a chapter which is eventually intended for the
 libvirt application development guide. It is not final yet, but I feel
 now would be a good moment to gather some first feedback and to
 finalise the XML schema which is used in the examples.

Thanks, this  is a good idea !

 
 
 
 1. Network Filter
 
   1.1. Overview
 
   1.2. XML Filter Description Format
 
 1.2.1. Complex Filter
 
 1.2.2. Simple Filters
 
   1.3. Retrieving Information About Filter
 
 1.3.1. TBD
 
 
 Chapter 1. Network Filter
 ---
 
 1.1. Overview
 
 1.2. XML Filter Description Format
 
   1.2.1. Complex Filter
 
   1.2.2. Simple Filters
 
 1.3. Retrieving Information About Filter
 
   1.3.1. TBD
 
 This section covers the management and definition of network filters
 using the libvirt API.
 
 
 
 
 1.2.2.Ă‚ Simple Filters
 
 The following examples of simple filters are predefined and address
 distint filter requirements. The predefined no-arp-spoofing filter drops
 all ARP packets
 
   *  originating from the guest if they contain other than the guests IP
 or MAC address
 
   *  destined for the guest if they contain other than the guests IP or
 MAC address
 
 It accepts all request or reply ARP packets.
 
   filter name='no-arp-spoofing' chain='arp'

Perhaps we should call that 'chain' attribute 'protocol' instead since
that appears to be what you're representing there. I'm wondering how
this should interact with the filterref element. eg, you might have
chain='ipv4' on the main filter, and then a filterref pointing to
a chain='arp'.  One way would be to declare that a filter can contain
either rule or filterref, but not a mixture of both.

uuidf88f1932-debf-4aa1-9fbe-f10d3aa4bc95/uuid

!-- no arp spoofing --
!-- drop if ipaddr or macaddr does not belong to guest --
rule action='drop' direction='out'
arp match='no' srcmacaddr='$MAC'/
/rule
rule action='drop' direction='out'
arp match='no' srcipaddr='$IP' /
/rule
!-- drop if ipaddr or macaddr odes not belong to guest --
rule action='drop' direction='in'
arp match='no' dstmacaddr='$MAC'/
/rule
rule action='drop' direction='in'
arp match='no' dstipaddr='$IP' /
/rule
!-- accept only request or reply packets --
rule action='accept' direction='inout'
arp opcode='request'/
/rule
rule action='accept' direction='inout'
arp opcode='reply'/
/rule
!-- drop everything else --
rule action='drop' direction='inout'/
 /filter


Generally, your proposal looks good to me.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] add an assert, to avoid a false-positive NULL-deref warning from clang

2010-03-01 Thread Jim Meyering
Here's a case in which using an assertion appears to be the only
way to tell clang that client really is non-NULL at that point.
I'm sure clang's analyzers will eventually improve, and hence avoid
this sort of false positive, so have marked this with a FIXME comment,
to help ensure we eventually remove this otherwise unnecessary assertion.

From ba26bad7aec8713ded0fd3300e951eac3673cc72 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 1 Mar 2010 15:19:48 +0100
Subject: [PATCH] add an assert, to avoid a false-positive NULL-deref warning 
from clang

* daemon/libvirtd.c (qemudWorker): Assert.
---
 daemon/libvirtd.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
index 9bdbecb..a357914 100644
--- a/daemon/libvirtd.c
+++ b/daemon/libvirtd.c
@@ -24,34 +24,35 @@
 #include config.h

 #include sys/types.h
 #include sys/wait.h
 #include sys/stat.h
 #include unistd.h
 #include fcntl.h
 #include limits.h
 #include sys/socket.h
 #include sys/un.h
 #include sys/poll.h
 #include netinet/in.h
 #include netinet/tcp.h
 #include netdb.h
 #include stdlib.h
 #include pwd.h
 #include stdio.h
+#include assert.h
 #include stdarg.h
 #include syslog.h
 #include string.h
 #include errno.h
 #include getopt.h
 #include fnmatch.h
 #include grp.h
 #include signal.h
 #include netdb.h

 #include libvirt_internal.h
 #include virterror_internal.h

 #define VIR_FROM_THIS VIR_FROM_QEMU

 #include libvirtd.h
 #include dispatch.h
@@ -1504,34 +1505,38 @@ static void *qemudWorker(void *data)
 virMutexLock(server-lock);
 while (((client = qemudPendingJob(server)) == NULL) 
!worker-quitRequest) {
 if (virCondWait(server-job, server-lock)  0) {
 virMutexUnlock(server-lock);
 return NULL;
 }
 }
 if (worker-quitRequest) {
 if (client)
 virMutexUnlock(client-lock);
 virMutexUnlock(server-lock);
 return NULL;
 }
 worker-processingCall = 1;
 virMutexUnlock(server-lock);

+/* Tell clang we know what we're doing.
+   FIXME: remove when clang improves.  */
+assert (client);
+
 /* We own a locked client now... */
 client-refs++;

 /* Remove our message from dispatch queue while we use it */
 msg = qemudClientMessageQueueServe(client-dx);

 /* This function drops the lock during dispatch,
  * and re-acquires it before returning */
 if (remoteDispatchClientRequest (server, client, msg)  0) {
 VIR_FREE(msg);
 qemudDispatchClientFailure(client);
 client-refs--;
 virMutexUnlock(client-lock);
 continue;
 }

 client-refs--;
--
1.7.0.1.414.g89213d

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


Re: [libvirt] [RFC PATCH] build: consistently use C99 varargs macros

2010-03-01 Thread Daniel Veillard
On Mon, Mar 01, 2010 at 08:04:35AM -0700, Eric Blake wrote:
 Prior to this patch, there was an inconsistent mix between GNU and
 C99.  For consistency, and potential portability to other compilers,
 stick with the C99 vararg macro syntax.
 
 * src/conf/cpu_conf.c (virCPUReportError): Use C99 rather than GNU
 vararg macro syntax.
 * src/conf/domain_conf.c (virDomainReportError): Likewise.
 * src/conf/domain_event.c (eventReportError): Likewise.
 * src/conf/interface_conf.c (virInterfaceReportError): Likewise.
 * src/conf/network_conf.c (virNetworkReportError): Likewise.
 * src/conf/node_device_conf.h (virNodeDeviceReportError):
 Likewise.
 * src/conf/secret_conf.h (virSecretReportError): Likewise.
 * src/conf/storage_conf.h (virStorageReportError): Likewise.
 ---
 
 Any objections to expanding this into a full-blown patch series?  This
 touches 8 files; I counted 43 more files that could be altered, using:
 $ git grep -l 'define.*[a-zA-Z]\.\.\.'
 
  src/conf/cpu_conf.c |4 ++--
  src/conf/domain_conf.c  |4 ++--
  src/conf/domain_event.c |5 +++--
  src/conf/interface_conf.c   |4 ++--
  src/conf/network_conf.c |6 +++---
  src/conf/node_device_conf.h |5 +++--
  src/conf/secret_conf.h  |6 +++---
  src/conf/storage_conf.h |6 +++---
  8 files changed, 21 insertions(+), 19 deletions(-)
 
 diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
 index ed83188..612e376 100644
 --- a/src/conf/cpu_conf.c
 +++ b/src/conf/cpu_conf.c
 @@ -31,9 +31,9 @@
 
  #define VIR_FROM_THIS VIR_FROM_CPU
 
 -#define virCPUReportError(code, fmt...)   \
 +#define virCPUReportError(code, ...)  \
  virReportErrorHelper(NULL, VIR_FROM_CPU, code, __FILE__,  \
 - __FUNCTION__, __LINE__, fmt)
 + __FUNCTION__, __LINE__, __VA_ARGS__)
 
  VIR_ENUM_IMPL(virCPUMatch, VIR_CPU_MATCH_LAST,
minimum,
 diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
 index 4420445..ea84863 100644
 --- a/src/conf/domain_conf.c
 +++ b/src/conf/domain_conf.c
 @@ -232,9 +232,9 @@ VIR_ENUM_IMPL(virDomainNetdevMacvtap, 
 VIR_DOMAIN_NETDEV_MACVTAP_MODE_LAST,
private,
bridge)
 
 -#define virDomainReportError(code, fmt...)   \
 +#define virDomainReportError(code, ...)  \
  virReportErrorHelper(NULL, VIR_FROM_DOMAIN, code, __FILE__,  \
 - __FUNCTION__, __LINE__, fmt)
 + __FUNCTION__, __LINE__, __VA_ARGS__)
 
  #ifndef PROXY
 
 diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
 index b474b1c..e791cab 100644
 --- a/src/conf/domain_event.c
 +++ b/src/conf/domain_event.c
 @@ -1,6 +1,7 @@
  /*
   * domain_event.c: domain event queue processing helpers
   *
 + * Copyright (C) 2010 Red Hat, Inc.
   * Copyright (C) 2008 VirtualIron
   *
   * This library is free software; you can redistribute it and/or
 @@ -30,9 +31,9 @@
 
  #define VIR_FROM_THIS VIR_FROM_NONE
 
 -#define eventReportError(conn, code, fmt...)\
 +#define eventReportError(conn, code, ...)   \
  virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__,   \
 - __FUNCTION__, __LINE__, fmt)
 + __FUNCTION__, __LINE__, __VA_ARGS__)
 
 
  /**
 diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
 index a0d2dfa..bc9e958 100644
 --- a/src/conf/interface_conf.c
 +++ b/src/conf/interface_conf.c
 @@ -45,9 +45,9 @@ static int
  virInterfaceDefDevFormat(virBufferPtr buf,
   const virInterfaceDefPtr def, int level);
 
 -#define virInterfaceReportError(code, fmt...)   \
 +#define virInterfaceReportError(code, ...)  \
  virReportErrorHelper(NULL, VIR_FROM_INTERFACE, code, __FILE__,  \
 - __FUNCTION__, __LINE__, fmt)
 + __FUNCTION__, __LINE__, __VA_ARGS__)
 
  static
  void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
 diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
 index 6d3c3c0..39ebd62 100644
 --- a/src/conf/network_conf.c
 +++ b/src/conf/network_conf.c
 @@ -1,7 +1,7 @@
  /*
   * network_conf.c: network XML handling
   *
 - * Copyright (C) 2006-2009 Red Hat, Inc.
 + * Copyright (C) 2006-2010 Red Hat, Inc.
   * Copyright (C) 2006-2008 Daniel P. Berrange
   *
   * This library is free software; you can redistribute it and/or
 @@ -53,9 +53,9 @@ VIR_ENUM_IMPL(virNetworkForward,
VIR_NETWORK_FORWARD_LAST,
none, nat, route )
 
 -#define virNetworkReportError(code, fmt...) \
 +#define virNetworkReportError(code, ...)\
  virReportErrorHelper(NULL, VIR_FROM_NETWORK, code, __FILE__,\
 - __FUNCTION__, __LINE__, fmt)
 

Re: [libvirt] [PATCH] add an assert, to avoid a false-positive NULL-deref warning from clang

2010-03-01 Thread Eric Blake
According to Jim Meyering on 3/1/2010 10:13 AM:
 Here's a case in which using an assertion appears to be the only
 way to tell clang that client really is non-NULL at that point.
 I'm sure clang's analyzers will eventually improve, and hence avoid
 this sort of false positive, so have marked this with a FIXME comment,
 to help ensure we eventually remove this otherwise unnecessary assertion.

Thanks for the extra context; it makes in-line review a breeze.

 @@ -1504,34 +1505,38 @@ static void *qemudWorker(void *data)
  virMutexLock(server-lock);
  while (((client = qemudPendingJob(server)) == NULL) 
 !worker-quitRequest) {
  if (virCondWait(server-job, server-lock)  0) {
  virMutexUnlock(server-lock);
  return NULL;
  }
  }

Indeed, the only way client can be NULL at this point is if
worker-quitRequest is true...

  if (worker-quitRequest) {
  if (client)
  virMutexUnlock(client-lock);
  virMutexUnlock(server-lock);
  return NULL;
  }

But that means we exit here.

  worker-processingCall = 1;
  virMutexUnlock(server-lock);
 
 +/* Tell clang we know what we're doing.
 +   FIXME: remove when clang improves.  */
 +assert (client);

So this assertion is valid.  ACK, if assert() is okay.

On the other hand, perhaps a more invasive rewrite would also work while
also avoiding assert(), by hoisting the worker-quitRequest into the while
loop, something like:

 while ((client = qemudPendingJob(server)) == NULL) {
 if (worker-quitRequest
 || virCondWait(server-job, server-lock)  0) {
 virMutexUnlock(server-lock);
 return NULL;
 }
 }
 if (worker-quitRequest) {
 virMutexUnlock(client-lock);
 virMutexUnlock(server-lock);
 return NULL;
 }

Should I write that into patch format?

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] add an assert, to avoid a false-positive NULL-deref warning from clang

2010-03-01 Thread Paolo Bonzini



On the other hand, perhaps a more invasive rewrite would also work while
also avoiding assert(), by hoisting the worker-quitRequest into the while
loop, something like:

  while ((client = qemudPendingJob(server)) == NULL) {
  if (worker-quitRequest
  || virCondWait(server-job,server-lock)  0) {
  virMutexUnlock(server-lock);
  return NULL;
  }
  }
  if (worker-quitRequest) {
  virMutexUnlock(client-lock);
  virMutexUnlock(server-lock);
  return NULL;
  }

Should I write that into patch format?


I think it's best to report the bug to clang's bugzilla and not clutter 
libvirt too much.


Paolo

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


Re: [libvirt] [RFC PATCH] build: consistently use C99 varargs macros

2010-03-01 Thread Paolo Bonzini

On 03/01/2010 06:24 PM, Daniel P. Berrange wrote:

mingw == gcc built as a cross-compiler, so there's no issue there.


Unfortunately, _native_ mingw (i.e. building under MSYS) uses a 
hideously old version of GCC (3.4.x), so it's not always a non-issue. 
But it is fine in this case:


http://gcc.gnu.org/onlinedocs/gcc-3.4.6/cpp/Variadic-Macros.html

Paolo

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


Re: [libvirt] [PATCH] add an assert, to avoid a false-positive NULL-deref warning from clang

2010-03-01 Thread Daniel Veillard
On Mon, Mar 01, 2010 at 10:27:43AM -0700, Eric Blake wrote:
 According to Jim Meyering on 3/1/2010 10:13 AM:
  Here's a case in which using an assertion appears to be the only
  way to tell clang that client really is non-NULL at that point.
  I'm sure clang's analyzers will eventually improve, and hence avoid
  this sort of false positive, so have marked this with a FIXME comment,
  to help ensure we eventually remove this otherwise unnecessary assertion.
 
 Thanks for the extra context; it makes in-line review a breeze.
 
  @@ -1504,34 +1505,38 @@ static void *qemudWorker(void *data)
   virMutexLock(server-lock);
   while (((client = qemudPendingJob(server)) == NULL) 
  !worker-quitRequest) {
   if (virCondWait(server-job, server-lock)  0) {
   virMutexUnlock(server-lock);
   return NULL;
   }
   }
 
 Indeed, the only way client can be NULL at this point is if
 worker-quitRequest is true...
 
   if (worker-quitRequest) {
   if (client)
   virMutexUnlock(client-lock);
   virMutexUnlock(server-lock);
   return NULL;
   }
 
 But that means we exit here.
 
   worker-processingCall = 1;
   virMutexUnlock(server-lock);
  
  +/* Tell clang we know what we're doing.
  +   FIXME: remove when clang improves.  */
  +assert (client);
 
 So this assertion is valid.  ACK, if assert() is okay.

  In general we really try to avoid assert() basically if we
exit that means that the clients loose their access to their VM
so that's a really really bad situation that we really try to avoid.
  Without refactoring assert() in that case would be acceptable
because basically that would mean the compiler generated broken code
and well, that something we don't try to gard against...

 On the other hand, perhaps a more invasive rewrite would also work while
 also avoiding assert(), by hoisting the worker-quitRequest into the while
 loop, something like:
 
  while ((client = qemudPendingJob(server)) == NULL) {
  if (worker-quitRequest
  || virCondWait(server-job, server-lock)  0) {
  virMutexUnlock(server-lock);
  return NULL;
  }
  }
  if (worker-quitRequest) {
  virMutexUnlock(client-lock);
  virMutexUnlock(server-lock);
  return NULL;
  }
 
 Should I write that into patch format?

  But if you can rewrite the loops to avoid the assert and keep clang
happy, I think it's an even better solution,

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] [PATCH 4/6] Allow a timezone to be specified instead of sync to host timezone

2010-03-01 Thread Daniel P. Berrange
On Mon, Mar 01, 2010 at 03:14:48PM +0100, Daniel Veillard wrote:
 On Thu, Feb 18, 2010 at 05:54:30PM +, Daniel P. Berrange wrote:
  This extends the XML to allow for
  
clock offset='timezone' timezone='Europe/Paris'/
  
  This is useful if the admin has not configured any timezone on the
  host OS, but still wants to synchronize a guest to a specific one.
  
  * src/conf/domain_conf.h, src/conf/domain_conf.c: Support extra
'timezone' attribute on clock configuration
  * docs/schemas/domain.rng: Add 'timezone' attribute
  * src/xen/xend_internal.c, src/xen/xm_internal.c: Reject configs
with a configurable timezone
  ---
   docs/schemas/domain.rng |   18 +++---
   src/conf/domain_conf.c  |   24 
   src/conf/domain_conf.h  |   13 ++---
   src/qemu/qemu_conf.c|2 +-
   src/xen/xend_internal.c |   10 --
   src/xen/xm_internal.c   |   17 -
   6 files changed, 66 insertions(+), 18 deletions(-)
  
  diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
  index d295bfe..4a36a97 100644
  --- a/docs/schemas/domain.rng
  +++ b/docs/schemas/domain.rng
  @@ -298,9 +298,16 @@
   optional
 element name=clock
  choice
  -  attribute name=offset
  -valuelocaltime/value
  - /attribute
  + group
  +attribute name=offset
  +  valuelocaltime/value
  +   /attribute
  +   optional
  + attribute name=timezone
  +   ref name=timeZone/
  + /attribute
  +   /optional
  + /group
 attribute name=offset
   valueutc/value
/attribute
  @@ -1584,4 +1591,9 @@
 param name=pattern(-|\+)?[0-9]+/param
   /data
 /define
  +  define name=timeZone
  +data type=string
  +  param name=pattern[a-zA-Z0-9_\.\+\-/]+/param
 
   Hum ... I wonder if we should not add ':' as it's used for POSIX TZ

How is it used ?  I looked at /usr/share/zoneinfo and all the files
there should be matched by this regex ok without needing ':'

Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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/6] Allow a timezone to be specified instead of sync to host timezone

2010-03-01 Thread Eric Blake
According to Daniel P. Berrange on 3/1/2010 11:45 AM:
 +data type=string
 +  param name=pattern[a-zA-Z0-9_\.\+\-/]+/param

   Hum ... I wonder if we should not add ':' as it's used for POSIX TZ
 
 How is it used ?  I looked at /usr/share/zoneinfo and all the files
 there should be matched by this regex ok without needing ':'

POSIX states that the use of ':' introduces implementation-defined extensions.

http://www.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap08.html

 The value of TZ has one of the two forms (spaces inserted for clarity):
 
 :characters
 
 or:
 
 std offset dst offset, rule
 
 If TZ is of the first format (that is, if the first character is a colon), 
 the characters following the colon are handled in an implementation-defined 
 manner.
 

In other words, since Europe/Paris is NOT a valid timezone according to
POSIX rules, a user concerned about POSIX compliance should be allowed to
use TZ=:Europe/Paris to still get glibc semantics of a named zone.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 2/6] Add new clock mode allowing variable adjustments

2010-03-01 Thread Jim Meyering
Daniel P. Berrange wrote:
 Here's the change to the syntax-check rule,
 followed by the change to the code to make those files comply.
 If everyone agrees, I'll reverse the order before pushing,
 so that make syntax-check passes after each commit.

 ACK, this makes sense - since we clearly keep re-introducing tabs
 in these files.

Thanks for the quick ACK.
Pushed.

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


[libvirt] [PATCH] x86Decode: avoid NULL-dereference upon questionable input

2010-03-01 Thread Jim Meyering
Clang warned about the potential NULL-dereference
via the STREQ/strcmp below.  models[i] could be NULL.
Even models could be NULL, and the allowed = ... test
makes that appear to be deliberately allowed.

The change below is the least invasive and cleanest
I could come up with, assuming there is no need to diagnose
(e.g., by returning -1) the condition of a NULL models[i] pointer.

From 623ac546a93f3e5b554647b05524b5041a642530 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Sun, 28 Feb 2010 13:34:06 +0100
Subject: [PATCH] x86Decode: avoid NULL-dereference upon questionable input

* src/cpu/cpu_x86.c (x86Decode): Don't dereference NULL when passed
a NULL models pointer, or when passed a nonzero nmodels value
and a corresponding NULL models[i].
---
 src/cpu/cpu_x86.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/cpu/cpu_x86.c b/src/cpu/cpu_x86.c
index 2194c32..b263629 100644
--- a/src/cpu/cpu_x86.c
+++ b/src/cpu/cpu_x86.c
@@ -957,89 +957,89 @@ x86Compare(virCPUDefPtr host,
virCPUDefPtr cpu)
 {
 return x86Compute(host, cpu, NULL);
 }


 static virCPUCompareResult
 x86GuestData(virCPUDefPtr host,
  virCPUDefPtr guest,
  union cpuData **data)
 {
 return x86Compute(host, guest, data);
 }


 static int
 x86Decode(virCPUDefPtr cpu,
   const union cpuData *data,
   const char **models,
   unsigned int nmodels)
 {
 int ret = -1;
 struct x86_map *map;
 const struct x86_model *candidate;
 virCPUDefPtr cpuCandidate;
 virCPUDefPtr cpuModel = NULL;
 struct cpuX86cpuid *cpuid;
 int i;

 if (data == NULL || (map = x86LoadMap()) == NULL)
 return -1;

 candidate = map-models;
 while (candidate != NULL) {
 bool allowed = (models == NULL);

 for (i = 0; i  candidate-ncpuid; i++) {
 cpuid = x86DataCpuid(data, candidate-cpuid[i].function);
 if (cpuid == NULL
 || !x86cpuidMatchMasked(cpuid, candidate-cpuid + i))
 goto next;
 }

 for (i = 0; i  nmodels; i++) {
-if (STREQ(models[i], candidate-name)) {
+if (models  models[i]  STREQ(models[i], candidate-name)) {
 allowed = true;
 break;
 }
 }

 if (!allowed) {
 VIR_DEBUG(CPU model %s not allowed by hypervisor; ignoring,
   candidate-name);
 goto next;
 }

 if (!(cpuCandidate = x86DataToCPU(data, candidate, map)))
 goto out;

 if (cpuModel == NULL
 || cpuModel-nfeatures  cpuCandidate-nfeatures) {
 virCPUDefFree(cpuModel);
 cpuModel = cpuCandidate;
 } else
 virCPUDefFree(cpuCandidate);

 next:
 candidate = candidate-next;
 }

 if (cpuModel == NULL) {
 virCPUReportError(VIR_ERR_INTERNAL_ERROR,
 %s, _(Cannot find suitable CPU model for given data));
 goto out;
 }

 cpu-model = cpuModel-model;
 cpu-nfeatures = cpuModel-nfeatures;
 cpu-features = cpuModel-features;
 VIR_FREE(cpuModel);

 ret = 0;

 out:
 x86MapFree(map);
 virCPUDefFree(cpuModel);

 return ret;
 }
--
1.7.0.1.414.g89213d

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


[libvirt] [PATCH] phypUUIDTable_Push: do not corrupt output stream

2010-03-01 Thread Jim Meyering
This started with clang's report of the two dead stores.
With the increment and decrement by sent being performed outside
of the intended loop, the two assignments had no net effect.

BTW, I presume these hard-coded names, from the same function,
are somehow temporary and/or debugging-related:

char remote_file[] = /home/hscroot/libvirt_uuid_table;
char local_file[] = ./uuid_table;


From a1104c4d1a95b10cab19bacf91ed073769f1d2d7 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 1 Mar 2010 21:26:59 +0100
Subject: [PATCH] phypUUIDTable_Push: do not corrupt output stream upon partial 
write

* src/phyp/phyp_driver.c (phypUUIDTable_Push): Move incr/decr
of ptr/nread into the loop where those variables are used.
Also, remove exit label and just-preceding goto.
---
 src/phyp/phyp_driver.c |7 ++-
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 1e8ed30..3fc5a0c 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -1982,54 +1982,51 @@ phypUUIDTable_Push(virConnectPtr conn)
 do {
 nread = fread(buffer, 1, sizeof(buffer), fd);
 if (nread = 0) {
 if (feof(fd)) {
 /* end of file */
 break;
 } else {
 VIR_ERROR(Failed to read from '%s', local_file);
 goto err;
 }
 }
 ptr = buffer;
 sent = 0;

 do {
 /* write the same data over and over, until error or completion */
 rc = libssh2_channel_write(channel, ptr, nread);
 if (LIBSSH2_ERROR_EAGAIN == rc) {   /* must loop around */
 continue;
 } else if (rc  0) {
 /* rc indicates how many bytes were written this time */
 sent += rc;
 }
+ptr += sent;
+nread -= sent;
 } while (rc  0  sent  nread);
-ptr += sent;
-nread -= sent;
 } while (1);

-goto exit;
-
-  exit:
 if (channel) {
 libssh2_channel_send_eof(channel);
 libssh2_channel_wait_eof(channel);
 libssh2_channel_wait_closed(channel);
 libssh2_channel_free(channel);
 channel = NULL;
 }
 return 0;

   err:
 if (channel) {
 libssh2_channel_send_eof(channel);
 libssh2_channel_wait_eof(channel);
 libssh2_channel_wait_closed(channel);
 libssh2_channel_free(channel);
 channel = NULL;
 }
 return -1;
 }

 int
 phypUUIDTable_Pull(virConnectPtr conn)
 {
--
1.7.0.1.414.g89213d

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


Re: [libvirt] [PATCH] x86Decode: avoid NULL-dereference upon questionable input

2010-03-01 Thread Eric Blake
According to Jim Meyering on 3/1/2010 1:21 PM:
 Clang warned about the potential NULL-dereference
 via the STREQ/strcmp below.  models[i] could be NULL.
 Even models could be NULL, and the allowed = ... test
 makes that appear to be deliberately allowed.

This same function was also listed by coverity, but only for models, not
models[i].

 The change below is the least invasive and cleanest
 I could come up with, assuming there is no need to diagnose
 (e.g., by returning -1) the condition of a NULL models[i] pointer.
 
  while (candidate != NULL) {
  bool allowed = (models == NULL);
 
  for (i = 0; i  candidate-ncpuid; i++) {
  cpuid = x86DataCpuid(data, candidate-cpuid[i].function);
  if (cpuid == NULL
  || !x86cpuidMatchMasked(cpuid, candidate-cpuid + i))
  goto next;
  }
 
  for (i = 0; i  nmodels; i++) {
 -if (STREQ(models[i], candidate-name)) {
 +if (models  models[i]  STREQ(models[i], candidate-name)) {

Isn't the intent that (models==NULL) iff (nmodels==0)?  In which case,
this code is unreachable if models is NULL.  But your patch certainly is
the least-invasive possible, and while it is only a false positive for
well-formed arguments, I didn't spend time checking all clients of
x86Decode to see if there is ever a possibility of bad arguments.

ACK

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] openvzDomainDefineCmd: remove useless increment

2010-03-01 Thread Eric Blake
According to Jim Meyering on 3/1/2010 1:41 PM:
 Another dead store.
 Refreshingly shallow.
 
 Subject: [PATCH] openvzDomainDefineCmd: remove useless increment
 
 * src/openvz/openvz_driver.c (openvzDomainDefineCmd): Remove
 useless increment of max_veid.

ACK.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH] phypUUIDTable_Push: do not corrupt output stream

2010-03-01 Thread Eric Blake
According to Jim Meyering on 3/1/2010 1:35 PM:
 This started with clang's report of the two dead stores.
 With the increment and decrement by sent being performed outside
 of the intended loop, the two assignments had no net effect.
 
  do {
  /* write the same data over and over, until error or completion 
 */
  rc = libssh2_channel_write(channel, ptr, nread);
  if (LIBSSH2_ERROR_EAGAIN == rc) {   /* must loop around */
  continue;
  } else if (rc  0) {
  /* rc indicates how many bytes were written this time */
  sent += rc;
  }
 +ptr += sent;
 +nread -= sent;
  } while (rc  0  sent  nread);
 -ptr += sent;
 -nread -= sent;

ACK.

  } while (1);
 
 -goto exit;
 -
 -  exit:

Definitely worth the cleanup.

-- 
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] inability to open local read-only connection

2010-03-01 Thread Tavares, John
Thanks for your suggestions Daniel!  I will rerun my executable thru strace 
with LIBVIRT_DEBUG=1 to see what happens and will send the results if need be.

John

-Original Message-
From: Daniel P. Berrange [mailto:berra...@redhat.com] 
Sent: Monday, March 01, 2010 7:58 AM
To: Tavares, John
Cc: libvir-list@redhat.com
Subject: Re: [libvirt] inability to open local read-only connection

On Thu, Feb 25, 2010 at 02:34:02PM -0600, Tavares, John wrote:
 I have been experimenting with using libvirt (0.3.3) on a variety of
 systems (RHEL, CentOS and Oracle VM).  I have run into an issue when
 I try to open a local read-only connection to the hypervisor that is
 failing only on Oracle VM server release 2.2.0.  I have created a root
 owned setuid executable that is effectively running as root, but even 
 so, still cannot open the local read-only connection of the hypervisor.
  It only works if I run it directly as root.  This is not an option.  I
 do not understand why it works as is on my RHEL and CentOS machines, but
 not my Oracle machine.  It would seem as thought it is not checking if
 the effective uid is root, just the uid.

A readonly connection essentially just goes to a separate UNIX socket, which
has more relaxed permissions (mode 0777, instead of 0700). The kernel does
the permissions checking when attempting to open it, so it should be using
the effective ID. 

 Has anyone run into a similar issue or have any suggestions of what I 
 might try to fix this issue or can tell me that this is a defect that 
 needs (is) fixed??

I'd suggest trying to strace your process  see where it fails, and/or run
with LIBVIRT_DEBUG=1 environemnt variable set.

Regards,
Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.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] [virt-tools-list] and for LXC?

2010-03-01 Thread Cleber Rosa
Hello there, 

libvirt already has a full feature driver for lxc. 

virt-install/virt-manager still need some love wrt lxc... I've started playing 
around with that, but that's still only a proof of concept. 

CR. 


--- 
Cleber Rodrigues  cr...@redhat.com  
Solutions Architect - Red Hat, Inc. 
Mobile: +55 61 9185.3454 

- Mensagem original - 
De: Mihamina Rakotomandimby miham...@gulfsat.mg 
Para: libvir-list@redhat.com, virt-tools-l...@redhat.com 
Enviadas: Segunda-feira, 1 de Março de 2010 10:14:34 (GMT-0300) Auto-Detected 
Assunto: [virt-tools-list] and for LXC? 

Manao ahoana, Hello, Bonjour, 

Is there some plan/schedule to make libvirt LXC friendly? 
Or is it already? 

Misaotra, Thanks, Merci. 

-- 
Architecte Informatique chez Blueline/Gulfsat: 
Administration Systeme, Recherche  Developpement 
+261 34 29 155 34 / +261 33 11 207 36 

___ 
virt-tools-list mailing list 
virt-tools-l...@redhat.com 
https://www.redhat.com/mailman/listinfo/virt-tools-list 
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Problems with attach-device/detach-device using libvirt 0.7.6

2010-03-01 Thread Rolf Eike Beer
I wrote:

[Problems attaching and detaching PCI devices]

Ok, today I was working on that again and USB attaching was found to be 
completely broken, too. Please drop my libvirt-0.7.6-null-pci-id.patch patch 
from the previous mail and use libvirt-0.7.6-null-device-id.patch instead: USB 
has the very same problem.

The read problem was indeed that attaching USB devices used pci-attach as 
qemu command instead of usb_add: libvirt-0.7.6-usb-attach.patch comes to 
help out. And I think it's rather confusing to get messages about errors in 
PCI device configuration when it's in fact an USB device. This had nothing to 
do with my problem, but I found that and libvirt-0.7.6-usb-messages.patch 
fixes it.

Eike
diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_conf.c libvirt-0.7.6/src/qemu/qemu_conf.c
--- libvirt-0.7.6-orig/src/qemu/qemu_conf.c	2010-02-03 17:56:38.0 +0100
+++ libvirt-0.7.6/src/qemu/qemu_conf.c	2010-03-01 16:35:21.0 +0100
@@ -2690,7 +2690,8 @@ qemuBuildPCIHostdevDevStr(virDomainHostd
   dev-source.subsys.u.pci.bus,
   dev-source.subsys.u.pci.slot,
   dev-source.subsys.u.pci.function);
-virBufferVSprintf(buf, ,id=%s, dev-info.alias);
+if (dev-info.alias != NULL)
+virBufferVSprintf(buf, ,id=%s, dev-info.alias);
 if (qemuBuildDeviceAddressStr(buf, dev-info)  0)
 goto error;
 
@@ -2734,11 +2735,12 @@ qemuBuildUSBHostdevDevStr(virDomainHostd
 return NULL;
 }
 
-if (virAsprintf(ret, usb-host,hostbus=%.3d,hostaddr=%.3d,id=%s,
+if (virAsprintf(ret, usb-host,hostbus=%.3d,hostaddr=%.3d,
 dev-source.subsys.u.usb.bus,
-dev-source.subsys.u.usb.device,
-dev-info.alias)  0)
+dev-source.subsys.u.usb.device)  0)
 virReportOOMError(NULL);
+if (dev-info.alias != NULL)
+virBufferVSprintf(ret, ,id=%s, dev-info.alias);
 
 return ret;
 }
diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_driver.c libvirt-0.7.6/src/qemu/qemu_driver.c
--- libvirt-0.7.6-orig/src/qemu/qemu_driver.c	2010-02-03 16:56:00.0 +0100
+++ libvirt-0.7.6/src/qemu/qemu_driver.c	2010-03-01 16:32:26.0 +0100
@@ -5831,7 +5831,7 @@ static int qemudDomainAttachHostUsbDevic
 char *devstr = NULL;
 
 if ((qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) 
-!(devstr = qemuBuildPCIHostdevDevStr(hostdev)))
+!(devstr = qemuBuildUSBHostdevDevStr(hostdev)))
 goto error;
 
 if (VIR_REALLOC_N(vm-def-hostdevs, vm-def-nhostdevs+1)  0) {
diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_conf.c libvirt-0.7.6/src/qemu/qemu_conf.c
--- libvirt-0.7.6-orig/src/qemu/qemu_conf.c	2010-02-03 17:56:38.0 +0100
+++ libvirt-0.7.6/src/qemu/qemu_conf.c	2010-03-01 16:35:21.0 +0100
@@ -4776,7 +4776,7 @@ qemuParseCommandLineUSB(virConnectPtr co
 
 if (!STRPREFIX(val, host:)) {
 qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _(unknown PCI device syntax '%s'), val);
+ _(unknown USB device syntax '%s'), val);
 VIR_FREE(def);
 goto cleanup;
 }
@@ -4792,21 +4792,21 @@ qemuParseCommandLineUSB(virConnectPtr co
 start = end + 1;
 if (virStrToLong_i(start, NULL, 16, second)  0) {
 qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _(cannot extract PCI device product '%s'), val);
+ _(cannot extract USB device product '%s'), val);
 VIR_FREE(def);
 goto cleanup;
 }
 } else {
 if (virStrToLong_i(start, end, 10, first)  0 || !end || *end != '.') {
 qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _(cannot extract PCI device bus '%s'), val);
+ _(cannot extract USB device bus '%s'), val);
 VIR_FREE(def);
 goto cleanup;
 }
 start = end + 1;
 if (virStrToLong_i(start, NULL, 10, second)  0) {
 qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _(cannot extract PCI device address '%s'), val);
+ _(cannot extract USB device address '%s'), val);
 VIR_FREE(def);
 goto cleanup;
 }


signature.asc
Description: This is a digitally signed message part.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] running libvirt on Xen Cloud Platform 0.7.1

2010-03-01 Thread Gadi Naor











Hi, 

I am trying to run libvirt daemon in Dom0 of Xen Cloud Platform 0.7.1.
For that purpose I downloaded the libvirt source files 0.7.6 and trying to 
build the libvirt rpm with XEN driver using the XCP DDK.

I am running into to many problems getting it done - so I was wondering whether 
this is supported or if anyone is using it in such way.

Thank You,
Gadi


  
_
Hotmail: Free, trusted and rich email service.
https://signup.live.com/signup.aspx?id=60969--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] Problems with attach-device/detach-device using libvirt 0.7.6

2010-03-01 Thread Daniel P. Berrange
On Mon, Mar 01, 2010 at 07:47:20PM +0100, Rolf Eike Beer wrote:
 I wrote:
 
 [Problems attaching and detaching PCI devices]
 
 Ok, today I was working on that again and USB attaching was found to be 
 completely broken, too. Please drop my libvirt-0.7.6-null-pci-id.patch patch 
 from the previous mail and use libvirt-0.7.6-null-device-id.patch instead: 
 USB 
 has the very same problem.
 
 The read problem was indeed that attaching USB devices used pci-attach as 
 qemu command instead of usb_add: libvirt-0.7.6-usb-attach.patch comes to 
 help out. And I think it's rather confusing to get messages about errors in 
 PCI device configuration when it's in fact an USB device. This had nothing to 
 do with my problem, but I found that and libvirt-0.7.6-usb-messages.patch 
 fixes it.
 
 Eike

 diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_conf.c 
 libvirt-0.7.6/src/qemu/qemu_conf.c
 --- libvirt-0.7.6-orig/src/qemu/qemu_conf.c   2010-02-03 17:56:38.0 
 +0100
 +++ libvirt-0.7.6/src/qemu/qemu_conf.c2010-03-01 16:35:21.0 
 +0100
 @@ -2690,7 +2690,8 @@ qemuBuildPCIHostdevDevStr(virDomainHostd
dev-source.subsys.u.pci.bus,
dev-source.subsys.u.pci.slot,
dev-source.subsys.u.pci.function);
 -virBufferVSprintf(buf, ,id=%s, dev-info.alias);
 +if (dev-info.alias != NULL)
 +virBufferVSprintf(buf, ,id=%s, dev-info.alias);
  if (qemuBuildDeviceAddressStr(buf, dev-info)  0)
  goto error;
  
 @@ -2734,11 +2735,12 @@ qemuBuildUSBHostdevDevStr(virDomainHostd
  return NULL;
  }
  
 -if (virAsprintf(ret, usb-host,hostbus=%.3d,hostaddr=%.3d,id=%s,
 +if (virAsprintf(ret, usb-host,hostbus=%.3d,hostaddr=%.3d,
  dev-source.subsys.u.usb.bus,
 -dev-source.subsys.u.usb.device,
 -dev-info.alias)  0)
 +dev-source.subsys.u.usb.device)  0)
  virReportOOMError(NULL);
 +if (dev-info.alias != NULL)
 +virBufferVSprintf(ret, ,id=%s, dev-info.alias);
  
  return ret;
  }

This patch isn't correct - all devices *must* have a alias assigned
before hotplug. The real issue here is the hotplug method is missing
the API call to assign the alias.


 diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_driver.c 
 libvirt-0.7.6/src/qemu/qemu_driver.c
 --- libvirt-0.7.6-orig/src/qemu/qemu_driver.c 2010-02-03 16:56:00.0 
 +0100
 +++ libvirt-0.7.6/src/qemu/qemu_driver.c  2010-03-01 16:32:26.0 
 +0100
 @@ -5831,7 +5831,7 @@ static int qemudDomainAttachHostUsbDevic
  char *devstr = NULL;
  
  if ((qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) 
 -!(devstr = qemuBuildPCIHostdevDevStr(hostdev)))
 +!(devstr = qemuBuildUSBHostdevDevStr(hostdev)))
  goto error;
  
  if (VIR_REALLOC_N(vm-def-hostdevs, vm-def-nhostdevs+1)  0) {

 diff -aurp libvirt-0.7.6-orig/src/qemu/qemu_conf.c 
 libvirt-0.7.6/src/qemu/qemu_conf.c
 --- libvirt-0.7.6-orig/src/qemu/qemu_conf.c   2010-02-03 17:56:38.0 
 +0100
 +++ libvirt-0.7.6/src/qemu/qemu_conf.c2010-03-01 16:35:21.0 
 +0100
 @@ -4776,7 +4776,7 @@ qemuParseCommandLineUSB(virConnectPtr co
  
  if (!STRPREFIX(val, host:)) {
  qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
 - _(unknown PCI device syntax '%s'), val);
 + _(unknown USB device syntax '%s'), val);
  VIR_FREE(def);
  goto cleanup;
  }
 @@ -4792,21 +4792,21 @@ qemuParseCommandLineUSB(virConnectPtr co
  start = end + 1;
  if (virStrToLong_i(start, NULL, 16, second)  0) {
  qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
 - _(cannot extract PCI device product '%s'), 
 val);
 + _(cannot extract USB device product '%s'), 
 val);
  VIR_FREE(def);
  goto cleanup;
  }
  } else {
  if (virStrToLong_i(start, end, 10, first)  0 || !end || *end != 
 '.') {
  qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
 - _(cannot extract PCI device bus '%s'), val);
 + _(cannot extract USB device bus '%s'), val);
  VIR_FREE(def);
  goto cleanup;
  }
  start = end + 1;
  if (virStrToLong_i(start, NULL, 10, second)  0) {
  qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
 - _(cannot extract PCI device address '%s'), 
 val);
 + _(cannot extract USB device address '%s'), 
 val);
  VIR_FREE(def);
  goto cleanup;
  }


These two look good  i'll apply them now.


Daniel
-- 
|: Red Hat, Engineering, London-o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :|
|: http://autobuild.org 

Re: [libvirt] [PATCH] x86Decode: avoid NULL-dereference upon questionable input

2010-03-01 Thread Jim Meyering
Eric Blake wrote:

 According to Jim Meyering on 3/1/2010 1:21 PM:
 Clang warned about the potential NULL-dereference
 via the STREQ/strcmp below.  models[i] could be NULL.
 Even models could be NULL, and the allowed = ... test
 makes that appear to be deliberately allowed.

 This same function was also listed by coverity, but only for models, not
 models[i].

Yes, I was disappointed to see Coverity missed that.

 The change below is the least invasive and cleanest
 I could come up with, assuming there is no need to diagnose
 (e.g., by returning -1) the condition of a NULL models[i] pointer.

  while (candidate != NULL) {
  bool allowed = (models == NULL);

  for (i = 0; i  candidate-ncpuid; i++) {
  cpuid = x86DataCpuid(data, candidate-cpuid[i].function);
  if (cpuid == NULL
  || !x86cpuidMatchMasked(cpuid, candidate-cpuid + i))
  goto next;
  }

  for (i = 0; i  nmodels; i++) {
 -if (STREQ(models[i], candidate-name)) {
 +if (models  models[i]  STREQ(models[i], candidate-name)) {

 Isn't the intent that (models==NULL) iff (nmodels==0)?

That is the intent, but the code at this level does not detect the mismatch.
I think someone made a change recently to protect us at a higher
(cpu-independent) level.
But that doesn't help us here, if a new caller of this function
violates those higher-level constraints.

 In which case,
 this code is unreachable if models is NULL.  But your patch certainly is
 the least-invasive possible, and while it is only a false positive for
 well-formed arguments, I didn't spend time checking all clients of
 x86Decode to see if there is ever a possibility of bad arguments.

 ACK

Thanks.

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


[libvirt] [PATCH] Fix USB/PCI device address aliases in QEMU hotplug driver

2010-03-01 Thread Daniel P. Berrange
The USB/PCI device hotplug code for the QEMU driver was forgetting
to allocate a unique device alias.

* src/qemu/qemu_driver.c: Fill in device alias for USB/PCI devices
---
 src/qemu/qemu_driver.c |   11 ---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c2bd46d..5ac3316 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -5878,6 +5878,8 @@ static int qemudDomainAttachHostPciDevice(struct 
qemud_driver *driver,
 }
 
 if (qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) {
+if (qemuAssignDeviceHostdevAlias(vm-def, hostdev, -1)  0)
+goto error;
 if (qemuDomainPCIAddressEnsureAddr(priv-pciaddrs, hostdev-info)  0)
 goto error;
 
@@ -5926,9 +5928,12 @@ static int qemudDomainAttachHostUsbDevice(struct 
qemud_driver *driver,
 qemuDomainObjPrivatePtr priv = vm-privateData;
 char *devstr = NULL;
 
-if ((qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) 
-!(devstr = qemuBuildUSBHostdevDevStr(hostdev)))
-goto error;
+if (qemuCmdFlags  QEMUD_CMD_FLAG_DEVICE) {
+if (qemuAssignDeviceHostdevAlias(vm-def, hostdev, -1)  0)
+goto error;
+if (!(devstr = qemuBuildUSBHostdevDevStr(hostdev)))
+goto error;
+}
 
 if (VIR_REALLOC_N(vm-def-hostdevs, vm-def-nhostdevs+1)  0) {
 virReportOOMError();
-- 
1.6.6.1

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


Re: [libvirt] [PATCH 4/6] Allow a timezone to be specified instead of sync to host timezone

2010-03-01 Thread Daniel Veillard
On Mon, Mar 01, 2010 at 06:45:28PM +, Daniel P. Berrange wrote:
 On Mon, Mar 01, 2010 at 03:14:48PM +0100, Daniel Veillard wrote:
  On Thu, Feb 18, 2010 at 05:54:30PM +, Daniel P. Berrange wrote:
   This extends the XML to allow for
   
 clock offset='timezone' timezone='Europe/Paris'/
   
   This is useful if the admin has not configured any timezone on the
   host OS, but still wants to synchronize a guest to a specific one.
   
   * src/conf/domain_conf.h, src/conf/domain_conf.c: Support extra
 'timezone' attribute on clock configuration
   * docs/schemas/domain.rng: Add 'timezone' attribute
   * src/xen/xend_internal.c, src/xen/xm_internal.c: Reject configs
 with a configurable timezone
   ---
docs/schemas/domain.rng |   18 +++---
src/conf/domain_conf.c  |   24 
src/conf/domain_conf.h  |   13 ++---
src/qemu/qemu_conf.c|2 +-
src/xen/xend_internal.c |   10 --
src/xen/xm_internal.c   |   17 -
6 files changed, 66 insertions(+), 18 deletions(-)
   
   diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng
   index d295bfe..4a36a97 100644
   --- a/docs/schemas/domain.rng
   +++ b/docs/schemas/domain.rng
   @@ -298,9 +298,16 @@
optional
  element name=clock
 choice
   -  attribute name=offset
   -valuelocaltime/value
   -   /attribute
   +   group
   +attribute name=offset
   +  valuelocaltime/value
   + /attribute
   + optional
   +   attribute name=timezone
   + ref name=timeZone/
   +   /attribute
   + /optional
   +   /group
  attribute name=offset
valueutc/value
   /attribute
   @@ -1584,4 +1591,9 @@
  param name=pattern(-|\+)?[0-9]+/param
/data
  /define
   +  define name=timeZone
   +data type=string
   +  param name=pattern[a-zA-Z0-9_\.\+\-/]+/param
  
Hum ... I wonder if we should not add ':' as it's used for POSIX TZ
 
 How is it used ?  I looked at /usr/share/zoneinfo and all the files
 there should be matched by this regex ok without needing ':'

  I looked there
http://www.gnu.org/s/libc/manual/html_node/TZ-Variable.html
seems that the offset allows for : separator for minutes/seconds
I'm sure there is at least some places not aligned on hourly boundaries :)
But honnestly it's not a big deal !

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] libvirt/dnsmasq integration.

2010-03-01 Thread Simon Kelley
Daniel P. Berrange wrote:

 The total set of DNSMASQ args that we currently use are

   --strict-order
   --bind-interfaces
   --domain DOMAIN-NAME(optional)
   --pid-file=/var/run/libvirt/network/$NETWORK.pid
   --conf-file=
   --listen-address=IPADDR-OF-BRIDGE
   --except-interface=lo
   --dhcp-range=IPRANGE(optional, multiple times)
   --dhcp-lease-max=RANGE-SIZE (optional)
   --dhcp-host=STATIC-HOST-MAPPING (optional)
   --enable-tftp   (optional)
   --tftp-root=/some/path  (optional)
   --dhcp-boot=PXE-BOOT-SERVER (optional)
 OK, the use of TFTP by libvirt is new to me. Looks like I'll have to
 pull some similar tricks to allow libvirt to enable TFTP on on interface
 without disturbing things elsewhere.
 
 We only very recently added the TFTP/boot options so it is probably
 not visible in many Linux distros yet.
 

 So, if we solve the extra TFTP interface binding issue, we shouldn't 
 need to worry about a formal 'scope', so lets ignore that suggestion
 of mine.
  


I propose that your dhcp-range should become

dhcp-range=interface:virt0, set:virt0tag, IPRANGE
dhcp-boot=tag:virt0tag,PXE_BOOT_SERVER

That engages the isolation mode for DHCP and ensures that the correct
PXE server is supplied to your clients even if a different one is
configured elsewhere. (The set: and tag: syntax is just new sugar
around the existing and rather confusing tagging code)

TFTP becomes.

enable-tftp=virt0

for a similar isolation mode, and

tftp-root=/some/path,virt0

to have  a root path which overrides one specified elsewhere.

Some preliminary code which implements the facilities required is at

http://www.thekelleys.org.uk/dnsmasq/test-releases/dnsmasq-2.53test12.tar.gz


Cheers,

Simon.

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


[libvirt] [PATCH 0/5] build: use C99 varargs

2010-03-01 Thread Eric Blake
Here's the full cleanup, based on my earlier RFC.  I've tested
that everything still builds and passes 'make syntax-check'.
This is broken out in chunks, although whoever ends up pushing
may want to squash it all into one.

 src/conf/cpu_conf.c|4 ++--
 src/conf/domain_conf.c |4 ++--
 src/conf/domain_event.c|5 +++--
 src/conf/interface_conf.c  |4 ++--
 src/conf/network_conf.c|6 +++---
 src/conf/node_device_conf.h|5 +++--
 src/conf/secret_conf.h |6 +++---
 src/conf/storage_conf.h|6 +++---
 src/cpu/cpu.h  |6 +++---
 src/datatypes.c|6 +++---
 src/esx/esx_device_monitor.c   |5 +++--
 src/esx/esx_driver.c   |5 +++--
 src/esx/esx_interface_driver.c |5 +++--
 src/esx/esx_network_driver.c   |5 +++--
 src/esx/esx_secret_driver.c|5 +++--
 src/esx/esx_storage_driver.c   |5 +++--
 src/esx/esx_util.c |5 +++--
 src/esx/esx_vi.c   |5 +++--
 src/esx/esx_vi_methods.c   |5 +++--
 src/esx/esx_vi_types.c |5 +++--
 src/esx/esx_vmx.c  |5 +++--
 src/interface/netcf_driver.c   |6 +++---
 src/libvirt.c  |6 +++---
 src/lxc/lxc_conf.h |5 +++--
 src/network/bridge_driver.c|4 ++--
 src/nodeinfo.c |6 +++---
 src/opennebula/one_conf.h  |8 +---
 src/openvz/openvz_conf.h   |5 +++--
 src/phyp/phyp_driver.c |5 +++--
 src/qemu/qemu_conf.h   |4 ++--
 src/remote/remote_driver.c |4 ++--
 src/security/security_driver.h |6 +++---
 src/test/test_driver.c |4 ++--
 src/uml/uml_conf.h |6 +++---
 src/util/hostusb.c |4 ++--
 src/util/json.c|6 +++---
 src/util/macvtap.c |5 +++--
 src/util/pci.c |4 ++--
 src/util/stats_linux.c |6 +++---
 src/util/util.c|4 ++--
 src/util/xml.c |6 +++---
 src/vbox/vbox_driver.c |5 +++--
 src/vbox/vbox_tmpl.c   |5 +++--
 src/xen/proxy_internal.c   |4 ++--
 src/xen/sexpr.c|9 -
 src/xen/xen_driver.c   |4 ++--
 src/xen/xen_hypervisor.c   |6 +++---
 src/xen/xen_inotify.c  |5 +++--
 src/xen/xend_internal.c|9 -
 src/xen/xm_internal.c  |6 +++---
 src/xen/xs_internal.c  |4 ++--
 51 files changed, 144 insertions(+), 124 deletions(-)


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


[libvirt] [PATCH 4/5] build: consistently use C99 varargs macros in xen

2010-03-01 Thread Eric Blake
Continuation of previous patch.

* src/xen/proxy_internal.c (virProxyError): Use C99 rather than
GNU vararg macro syntax.
* src/xen/sexpr.c (virSexprError): Likewise.
* src/xen/xen_driver.c (xenUnifiedError): Likewise.
* src/xen/xen_hypervisor.c (virXenError): Likewise.
* src/xen/xen_inotify.c (virXenInotifyError): Likewise.
* src/xen/xend_internal.c (virXendError): Likewise.
* src/xen/xm_internal.c (xenXMError): Likewise.
* src/xen/xs_internal.c (virXenStoreError): Likewise.
---
 src/xen/proxy_internal.c |4 ++--
 src/xen/sexpr.c  |9 -
 src/xen/xen_driver.c |4 ++--
 src/xen/xen_hypervisor.c |6 +++---
 src/xen/xen_inotify.c|5 +++--
 src/xen/xend_internal.c  |9 -
 src/xen/xm_internal.c|6 +++---
 src/xen/xs_internal.c|4 ++--
 8 files changed, 23 insertions(+), 24 deletions(-)

diff --git a/src/xen/proxy_internal.c b/src/xen/proxy_internal.c
index 8e1c226..109b99b 100644
--- a/src/xen/proxy_internal.c
+++ b/src/xen/proxy_internal.c
@@ -92,9 +92,9 @@ struct xenUnifiedDriver xenProxyDriver = {
  * *
  /

-#define virProxyError(conn, code, fmt...)\
+#define virProxyError(conn, code, ...) \
 virReportErrorHelper(conn, VIR_FROM_PROXY, code, __FILE__, \
-   __FUNCTION__, __LINE__, fmt)
+   __FUNCTION__, __LINE__, __VA_ARGS__)

 /
  * *
diff --git a/src/xen/sexpr.c b/src/xen/sexpr.c
index 840d3e5..7e370db 100644
--- a/src/xen/sexpr.c
+++ b/src/xen/sexpr.c
@@ -1,9 +1,8 @@
 /*
  * sexpr.c : S-Expression routines to communicate with the Xen Daemon
  *
- * Copyright (C) 2005
- *
- *  Anthony Liguori aligu...@us.ibm.com
+ * Copyright (C) 2010 Red Hat, Inc.
+ * Copyright (C) 2005 Anthony Liguori aligu...@us.ibm.com
  *
  *  This file is subject to the terms and conditions of the GNU Lesser General
  *  Public License. See the file COPYING.LIB in the main directory of this
@@ -25,9 +24,9 @@

 #define VIR_FROM_THIS VIR_FROM_SEXPR

-#define virSexprError(code, fmt...)  \
+#define virSexprError(code, ...)   \
 virReportErrorHelper(NULL, VIR_FROM_SEXPR, code, __FILE__, \
-   __FUNCTION__, __LINE__, fmt)
+   __FUNCTION__, __LINE__, __VA_ARGS__)

 /**
  * sexpr_new:
diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 700682c..eac485f 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -72,9 +72,9 @@ static struct xenUnifiedDriver const * const 
drivers[XEN_UNIFIED_NR_DRIVERS] = {

 static int inside_daemon;

-#define xenUnifiedError(conn, code, fmt...)  \
+#define xenUnifiedError(conn, code, ...)   \
 virReportErrorHelper(conn, VIR_FROM_XEN, code, __FILE__,   \
-   __FUNCTION__, __LINE__, fmt)
+   __FUNCTION__, __LINE__, __VA_ARGS__)

 /**
  * xenNumaInit:
diff --git a/src/xen/xen_hypervisor.c b/src/xen/xen_hypervisor.c
index ab8fbc3..fe49ac2 100644
--- a/src/xen/xen_hypervisor.c
+++ b/src/xen/xen_hypervisor.c
@@ -803,10 +803,10 @@ struct xenUnifiedDriver xenHypervisorDriver = {
 };
 #endif /* !PROXY */

-#define virXenError(conn, code, fmt...)  \
-if (in_init == 0)\
+#define virXenError(conn, code, ...)   \
+if (in_init == 0)  \
 virReportErrorHelper(conn, VIR_FROM_XEN, code, __FILE__,   \
-   __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 #ifndef PROXY

diff --git a/src/xen/xen_inotify.c b/src/xen/xen_inotify.c
index de91dc6..ee24bc4 100644
--- a/src/xen/xen_inotify.c
+++ b/src/xen/xen_inotify.c
@@ -4,6 +4,7 @@
  */etc/xen
  */var/lib/xend/domains
  *
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2008 VirtualIron
  *
  * This library is free software; you can redistribute it and/or
@@ -43,9 +44,9 @@

 #define VIR_FROM_THIS VIR_FROM_XEN_INOTIFY

-#define virXenInotifyError(conn, code, fmt...) 
\
+#define virXenInotifyError(conn, code, ...)   \
 virReportErrorHelper(NULL, VIR_FROM_XEN_INOTIFY, code, __FILE__,  \
-   __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, 

[libvirt] [PATCH 1/5] build: consistently use C99 varargs macros in conf

2010-03-01 Thread Eric Blake
Prior to this patch, there was an inconsistent mix between GNU:

and C99:

For consistency, and potential portability to other compilers, stick
with the C99 vararg macro syntax.

* src/conf/cpu_conf.c (virCPUReportError): Use C99 rather than GNU
vararg macro syntax.
* src/conf/domain_conf.c (virDomainReportError): Likewise.
* src/conf/domain_event.c (eventReportError): Likewise.
* src/conf/interface_conf.c (virInterfaceReportError): Likewise.
* src/conf/network_conf.c (virNetworkReportError): Likewise.
* src/conf/node_device_conf.h (virNodeDeviceReportError):
Likewise.
* src/conf/secret_conf.h (virSecretReportError): Likewise.
* src/conf/storage_conf.h (virStorageReportError): Likewise.
---
 src/conf/cpu_conf.c |4 ++--
 src/conf/domain_conf.c  |4 ++--
 src/conf/domain_event.c |5 +++--
 src/conf/interface_conf.c   |4 ++--
 src/conf/network_conf.c |6 +++---
 src/conf/node_device_conf.h |5 +++--
 src/conf/secret_conf.h  |6 +++---
 src/conf/storage_conf.h |6 +++---
 8 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c
index ed83188..612e376 100644
--- a/src/conf/cpu_conf.c
+++ b/src/conf/cpu_conf.c
@@ -31,9 +31,9 @@

 #define VIR_FROM_THIS VIR_FROM_CPU

-#define virCPUReportError(code, fmt...)   \
+#define virCPUReportError(code, ...)  \
 virReportErrorHelper(NULL, VIR_FROM_CPU, code, __FILE__,  \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 VIR_ENUM_IMPL(virCPUMatch, VIR_CPU_MATCH_LAST,
   minimum,
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index df1ec18..96ba0b0 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -238,9 +238,9 @@ VIR_ENUM_IMPL(virDomainClockOffset, 
VIR_DOMAIN_CLOCK_OFFSET_LAST,
   variable,
   timezone);

-#define virDomainReportError(code, fmt...)   \
+#define virDomainReportError(code, ...)  \
 virReportErrorHelper(NULL, VIR_FROM_DOMAIN, code, __FILE__,  \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 #ifndef PROXY

diff --git a/src/conf/domain_event.c b/src/conf/domain_event.c
index b474b1c..e791cab 100644
--- a/src/conf/domain_event.c
+++ b/src/conf/domain_event.c
@@ -1,6 +1,7 @@
 /*
  * domain_event.c: domain event queue processing helpers
  *
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2008 VirtualIron
  *
  * This library is free software; you can redistribute it and/or
@@ -30,9 +31,9 @@

 #define VIR_FROM_THIS VIR_FROM_NONE

-#define eventReportError(conn, code, fmt...)\
+#define eventReportError(conn, code, ...)   \
 virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__,   \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)


 /**
diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index a0d2dfa..bc9e958 100644
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -45,9 +45,9 @@ static int
 virInterfaceDefDevFormat(virBufferPtr buf,
  const virInterfaceDefPtr def, int level);

-#define virInterfaceReportError(code, fmt...)   \
+#define virInterfaceReportError(code, ...)  \
 virReportErrorHelper(NULL, VIR_FROM_INTERFACE, code, __FILE__,  \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 static
 void virInterfaceIpDefFree(virInterfaceIpDefPtr def) {
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 6d3c3c0..39ebd62 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1,7 +1,7 @@
 /*
  * network_conf.c: network XML handling
  *
- * Copyright (C) 2006-2009 Red Hat, Inc.
+ * Copyright (C) 2006-2010 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -53,9 +53,9 @@ VIR_ENUM_IMPL(virNetworkForward,
   VIR_NETWORK_FORWARD_LAST,
   none, nat, route )

-#define virNetworkReportError(code, fmt...) \
+#define virNetworkReportError(code, ...)\
 virReportErrorHelper(NULL, VIR_FROM_NETWORK, code, __FILE__,\
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 virNetworkObjPtr virNetworkFindByUUID(const virNetworkObjListPtr nets,
   const unsigned char *uuid)
diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h
index 3a5432c..cbaad9b 100644
--- a/src/conf/node_device_conf.h
+++ b/src/conf/node_device_conf.h

[libvirt] [PATCH 5/5] build: consistently use C99 varargs macros throughout

2010-03-01 Thread Eric Blake
Conclusion of previous patches.

* src/cpu/cpu.h (virCPUReportError): Use C99 rather than GNU
vararg macro syntax.
* src/datatypes.c (virLibConnError): Likewise.
* src/interface/netcf_driver.c (interfaceReportError): Likewise.
* src/libvirt.c (virLibStreamError): Likewise.
* src/lxc/lxc_conf.h (lxcError): Likewise.
* src/network/bridge_driver.c (networkReportError): Likewise.
* src/nodeinfo.c (nodeReportError): Likewise.
* src/opennebula/one_conf.h (oneError): Likewise.
* src/openvz/openvz_conf.h (openvzError): Likewise.
* src/phyp/phyp_driver.c (PHYP_ERROR): Likewise.
* src/qemu/qemu_conf.h (qemuReportError): Likewise.
* src/remote/remote_driver.c (errorf): Likewise.
* src/security/security_driver.h (virSecurityReportError):
Likewise.
* src/test/test_driver.c (testError): Likewise.
* src/uml/uml_conf.h (umlReportError): Likewise.
* src/vbox/vbox_driver.c (vboxError): Likewise.
* src/vbox/vbox_tmpl.c (vboxError): Likewise.
---
 src/cpu/cpu.h  |6 +++---
 src/datatypes.c|6 +++---
 src/interface/netcf_driver.c   |6 +++---
 src/libvirt.c  |6 +++---
 src/lxc/lxc_conf.h |5 +++--
 src/network/bridge_driver.c|4 ++--
 src/nodeinfo.c |6 +++---
 src/opennebula/one_conf.h  |8 +---
 src/openvz/openvz_conf.h   |5 +++--
 src/phyp/phyp_driver.c |5 +++--
 src/qemu/qemu_conf.h   |4 ++--
 src/remote/remote_driver.c |4 ++--
 src/security/security_driver.h |6 +++---
 src/test/test_driver.c |4 ++--
 src/uml/uml_conf.h |6 +++---
 src/vbox/vbox_driver.c |5 +++--
 src/vbox/vbox_tmpl.c   |5 +++--
 17 files changed, 49 insertions(+), 42 deletions(-)

diff --git a/src/cpu/cpu.h b/src/cpu/cpu.h
index c8d961d..4287ca3 100644
--- a/src/cpu/cpu.h
+++ b/src/cpu/cpu.h
@@ -1,7 +1,7 @@
 /*
  * cpu.h: internal functions for CPU manipulation
  *
- * Copyright (C) 2009--2010 Red Hat, Inc.
+ * Copyright (C) 2009-2010 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -30,9 +30,9 @@
 #include cpu_x86_data.h


-#define virCPUReportError(code, fmt...)   \
+#define virCPUReportError(code, ...)  \
 virReportErrorHelper(NULL, VIR_FROM_CPU, code, __FILE__,  \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)


 union cpuData {
diff --git a/src/datatypes.c b/src/datatypes.c
index fa342e0..01601d8 100644
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -1,7 +1,7 @@
 /*
  * datatypes.h: management of structs for public data types
  *
- * Copyright (C) 2006-2009 Red Hat, Inc.
+ * Copyright (C) 2006-2010 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -31,9 +31,9 @@

 #define VIR_FROM_THIS VIR_FROM_NONE

-#define virLibConnError(conn, code, fmt...)   \
+#define virLibConnError(conn, code, ...)  \
 virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__, \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 /
  * *
diff --git a/src/interface/netcf_driver.c b/src/interface/netcf_driver.c
index 7f4d43d..e44156e 100644
--- a/src/interface/netcf_driver.c
+++ b/src/interface/netcf_driver.c
@@ -2,7 +2,7 @@
  * interface_driver.c: backend driver methods to handle physical
  * interface configuration using the netcf library.
  *
- * Copyright (C) 2006-2009 Red Hat, Inc.
+ * Copyright (C) 2006-2010 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -33,9 +33,9 @@

 #define VIR_FROM_THIS VIR_FROM_INTERFACE

-#define interfaceReportError(conn, dom, net, code, fmt...)\
+#define interfaceReportError(conn, dom, net, code, ...)   \
 virReportErrorHelper(conn, VIR_FROM_THIS, code, __FILE__, \
-   __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 /* Main driver state */
 struct interface_driver
diff --git a/src/libvirt.c b/src/libvirt.c
index 9d50c92..b664728 100644
--- a/src/libvirt.c
+++ b/src/libvirt.c
@@ -2,7 +2,7 @@
  * libvirt.c: Main interfaces for the libvirt library to handle virtualization
  *   domains from a process running in domain 0
  *
- * Copyright (C) 2005,2006,2008,2009 Red Hat, Inc.
+ * Copyright (C) 2005-2006, 2008-2010 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
@@ -618,9 +618,9 @@ 

[libvirt] [PATCH 2/5] build: consistently use C99 varargs macros in esx

2010-03-01 Thread Eric Blake
Continuation of previous patch.

* src/esx/esx_device_monitor.c (ESX_ERROR): Use C99 rather than
GNU vararg macro syntax.
* src/esx/esx_driver.c (ESX_ERROR): Likewise.
* src/esx/esx_interface_driver.c (ESX_ERROR): Likewise.
* src/esx/esx_network_driver.c (ESX_ERROR): Likewise.
* src/esx/esx_secret_driver.c (ESX_ERROR): Likewise.
* src/esx/esx_storage_driver.c (ESX_ERROR): Likewise.
* src/esx/esx_util.c (ESX_ERROR): Likewise.
* src/esx/esx_vi.c (ESX_VI_ERROR): Likewise.
* src/esx/esx_vi_methods.c (ESX_VI_ERROR): Likewise.
* src/esx/esx_vi_types.c (ESX_VI_ERROR): Likewise.
* src/esx/esx_vmx.c (ESX_ERROR): Likewise.
---
 src/esx/esx_device_monitor.c   |5 +++--
 src/esx/esx_driver.c   |5 +++--
 src/esx/esx_interface_driver.c |5 +++--
 src/esx/esx_network_driver.c   |5 +++--
 src/esx/esx_secret_driver.c|5 +++--
 src/esx/esx_storage_driver.c   |5 +++--
 src/esx/esx_util.c |5 +++--
 src/esx/esx_vi.c   |5 +++--
 src/esx/esx_vi_methods.c   |5 +++--
 src/esx/esx_vi_types.c |5 +++--
 src/esx/esx_vmx.c  |5 +++--
 11 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/src/esx/esx_device_monitor.c b/src/esx/esx_device_monitor.c
index de67ebb..78de0e0 100644
--- a/src/esx/esx_device_monitor.c
+++ b/src/esx/esx_device_monitor.c
@@ -3,6 +3,7 @@
  * esx_device_monitor.c: device monitor methods for managing VMware ESX
  *   host devices
  *
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2010 Matthias Bolte matthias.bo...@googlemail.com
  *
  * This library is free software; you can redistribute it and/or
@@ -37,9 +38,9 @@

 #define VIR_FROM_THIS VIR_FROM_ESX

-#define ESX_ERROR(conn, code, fmt...) \
+#define ESX_ERROR(conn, code, ...)\
 virReportErrorHelper(conn, VIR_FROM_ESX, code, __FILE__, __FUNCTION__,\
- __LINE__, fmt)
+ __LINE__, __VA_ARGS__)



diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index e125a09..2b52884 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -2,6 +2,7 @@
 /*
  * esx_driver.c: core driver methods for managing VMware ESX hosts
  *
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2009, 2010 Matthias Bolte matthias.bo...@googlemail.com
  * Copyright (C) 2009 Maximilian Wilhelm m...@rfc2324.org
  *
@@ -46,9 +47,9 @@

 #define VIR_FROM_THIS VIR_FROM_ESX

-#define ESX_ERROR(code, fmt...)   \
+#define ESX_ERROR(code, ...)  \
 virReportErrorHelper(NULL, VIR_FROM_ESX, code, __FILE__, __FUNCTION__,\
- __LINE__, fmt)
+ __LINE__, __VA_ARGS__)

 static int esxDomainGetMaxVcpus(virDomainPtr domain);

diff --git a/src/esx/esx_interface_driver.c b/src/esx/esx_interface_driver.c
index 6e9d224..e3739f7 100644
--- a/src/esx/esx_interface_driver.c
+++ b/src/esx/esx_interface_driver.c
@@ -3,6 +3,7 @@
  * esx_interface_driver.h: interface driver methods for managing VMware ESX
  * host interfaces
  *
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2010 Matthias Bolte matthias.bo...@googlemail.com
  *
  * This library is free software; you can redistribute it and/or
@@ -37,9 +38,9 @@

 #define VIR_FROM_THIS VIR_FROM_ESX

-#define ESX_ERROR(conn, code, fmt...) \
+#define ESX_ERROR(conn, code, ...)\
 virReportErrorHelper(conn, VIR_FROM_ESX, code, __FILE__, __FUNCTION__,\
- __LINE__, fmt)
+ __LINE__, __VA_ARGS__)



diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
index 1eeae44..dacc95b 100644
--- a/src/esx/esx_network_driver.c
+++ b/src/esx/esx_network_driver.c
@@ -3,6 +3,7 @@
  * esx_network_driver.c: network driver methods for managing VMware ESX
  *   host networks
  *
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2010 Matthias Bolte matthias.bo...@googlemail.com
  *
  * This library is free software; you can redistribute it and/or
@@ -37,9 +38,9 @@

 #define VIR_FROM_THIS VIR_FROM_ESX

-#define ESX_ERROR(conn, code, fmt...) \
+#define ESX_ERROR(conn, code, ...)\
 virReportErrorHelper(conn, VIR_FROM_ESX, code, __FILE__, __FUNCTION__,\
- __LINE__, fmt)
+ __LINE__, __VA_ARGS__)



diff --git a/src/esx/esx_secret_driver.c b/src/esx/esx_secret_driver.c
index 7b24d0c..07a178f 100644
--- a/src/esx/esx_secret_driver.c
+++ b/src/esx/esx_secret_driver.c
@@ -2,6 +2,7 @@
 /*
  * esx_secret_driver.c: secret driver methods for VMware ESX secret 
manipulation
  *
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2010 

[libvirt] [PATCH 3/5] build: consistently use C99 varargs macros in util

2010-03-01 Thread Eric Blake
Continuation of previous patch.

* src/util/hostusb.c (usbReportError): Use C99 rather than GNU
vararg macro syntax.
* src/util/json.c (virJSONError): Likewise.
* src/util/macvtap.c (ReportError): Likewise.
* src/util/pci.c (pciReportError): Likewise.
* src/util/stats_linux.c (virStatsError): Likewise.
* src/util/util.c (virUtilError): Likewise.
* src/util/xml.c (virXMLError): Likewise.
---
 src/util/hostusb.c |4 ++--
 src/util/json.c|6 +++---
 src/util/macvtap.c |5 +++--
 src/util/pci.c |4 ++--
 src/util/stats_linux.c |6 +++---
 src/util/util.c|4 ++--
 src/util/xml.c |6 +++---
 7 files changed, 18 insertions(+), 17 deletions(-)

diff --git a/src/util/hostusb.c b/src/util/hostusb.c
index bf96539..4f57b59 100644
--- a/src/util/hostusb.c
+++ b/src/util/hostusb.c
@@ -54,9 +54,9 @@ struct _usbDevice {
 /* For virReportOOMError()  and virReportSystemError() */
 #define VIR_FROM_THIS VIR_FROM_NONE

-#define usbReportError(code, fmt...)   \
+#define usbReportError(code, ...)  \
 virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__,  \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 static int usbSysReadFile(const char *f_name, const char *d_name,
   int base, unsigned *value)
diff --git a/src/util/json.c b/src/util/json.c
index 3abff58..e388287 100644
--- a/src/util/json.c
+++ b/src/util/json.c
@@ -1,8 +1,8 @@
 /*
  * json.c: JSON object parsing/formatting
  *
- * Copyright (C) 2009 Daniel P. Berrange
  * Copyright (C) 2009-2010 Red Hat, Inc.
+ * Copyright (C) 2009 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -36,9 +36,9 @@

 /* XXX fixme */
 #define VIR_FROM_THIS VIR_FROM_NONE
-#define virJSONError(code, fmt...)  \
+#define virJSONError(code, ...) \
 virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__,   \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)


 typedef struct _virJSONParserState virJSONParserState;
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 8c29588..3b526ce 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -1,4 +1,5 @@
 /*
+ * Copyright (C) 2010 Red Hat, Inc.
  * Copyright (C) 2010 IBM Corporation
  *
  * This library is free software; you can redistribute it and/or
@@ -48,9 +49,9 @@

 #define VIR_FROM_THIS VIR_FROM_NET

-#define ReportError(conn, code, fmt...)  \
+#define ReportError(conn, code, ...)  \
 virReportErrorHelper(conn, VIR_FROM_NET, code, __FILE__,  \
-   __FUNCTION__, __LINE__, fmt)
+   __FUNCTION__, __LINE__, __VA_ARGS__)

 #define MACVTAP_NAME_PREFIXmacvtap
 #define MACVTAP_NAME_PATTERN   macvtap%d
diff --git a/src/util/pci.c b/src/util/pci.c
index b812241..c6b2072 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -75,9 +75,9 @@ struct _pciDeviceList {
 /* For virReportOOMError()  and virReportSystemError() */
 #define VIR_FROM_THIS VIR_FROM_NONE

-#define pciReportError(code, fmt...)   \
+#define pciReportError(code, ...)  \
 virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__,  \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)

 /* Specifications referenced in comments:
  *  PCI30  - PCI Local Bus Specification 3.0
diff --git a/src/util/stats_linux.c b/src/util/stats_linux.c
index 25f8d08..feb5589 100644
--- a/src/util/stats_linux.c
+++ b/src/util/stats_linux.c
@@ -1,7 +1,7 @@
 /*
  * Linux block and network stats.
  *
- * Copyright (C) 2007-2009 Red Hat, Inc.
+ * Copyright (C) 2007-2010 Red Hat, Inc.
  *
  * See COPYING.LIB for the License of this software
  *
@@ -28,9 +28,9 @@

 #define VIR_FROM_THIS VIR_FROM_STATS_LINUX

-#define virStatsError(code, fmt...)\
+#define virStatsError(code, ...)   \
 virReportErrorHelper(NULL, VIR_FROM_THIS, code, __FILE__,  \
- __FUNCTION__, __LINE__, fmt)
+ __FUNCTION__, __LINE__, __VA_ARGS__)


 /* interface stats */
diff --git a/src/util/util.c b/src/util/util.c
index cf7bba5..abead4a 100644
--- a/src/util/util.c
+++ b/src/util/util.c
@@ -85,9 +85,9 @@

 #define VIR_FROM_THIS VIR_FROM_NONE

-#define virUtilError(code, fmt...) \
+#define virUtilError(code, ...)\
 virReportErrorHelper(NULL, VIR_FROM_NONE, code, __FILE__,

[libvirt] [PATCH] Change default for storage uid/gid from getuid()/getgid() to -1/-1

2010-03-01 Thread Laine Stump
This allows the config to have a setting that means leave it alone,
eg when building a pool where the directory already exists the user
may want the current uid/gid of the directory left intact. This
actually gets us back to older behavior - before recent changes to the
pool building code, we weren't as insistent about honoring the uid/gid
settings in the XML, and virt-manager was taking advantage of this
behavior.

As a side benefit, removing calls to getuid/getgid from the XML
parsing functions also seems like a good idea. And having a default
that is different from a common/useful value (0 == root) is a good
thing in general, as it removes ambiguity from decisions (at least one
place in the code was checking for (perms.uid == 0) to see if a
special uid was requested).

Note that this will only affect newly created pools and volumes. Due
to the way that the XML is parsed, then formatted for newly created
volumes, all existing pools/volumes already have an explicit uid and
gid set.

src/conf/storage_conf.c: Remove calls to setuid/setgid for default values
 of uid/gid, and set them to -1 instead

src/storage/storage_backend.c:
src/storage/storage_backend_fs.c:
Make account for the new default values of perms.uid
and perms.gid.
---
 src/conf/storage_conf.c  |8 +++---
 src/storage/storage_backend.c|   25 +--
 src/storage/storage_backend_fs.c |   39 +++--
 3 files changed, 46 insertions(+), 26 deletions(-)

diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 19a1db9..f59f109 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -539,8 +539,8 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
 if (node == NULL) {
 /* Set default values if there is not permissions element */
 perms-mode = defaultmode;
-perms-uid = getuid();
-perms-gid = getgid();
+perms-uid = -1;
+perms-gid = -1;
 perms-label = NULL;
 return 0;
 }
@@ -564,7 +564,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
 }
 
 if (virXPathNode(./owner, ctxt) == NULL) {
-perms-uid = getuid();
+perms-uid = -1;
 } else {
 if (virXPathLong(number(./owner), ctxt, v)  0) {
 virStorageReportError(VIR_ERR_XML_ERROR,
@@ -575,7 +575,7 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt,
 }
 
 if (virXPathNode(./group, ctxt) == NULL) {
-perms-gid = getgid();
+perms-gid = -1;
 } else {
 if (virXPathLong(number(./group), ctxt, v)  0) {
 virStorageReportError(VIR_ERR_XML_ERROR,
diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c
index 3742493..ee6a32e 100644
--- a/src/storage/storage_backend.c
+++ b/src/storage/storage_backend.c
@@ -241,11 +241,10 @@ virStorageBackendCreateBlockFrom(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 uid = (vol-target.perms.uid != st.st_uid) ? vol-target.perms.uid : -1;
 gid = (vol-target.perms.gid != st.st_gid) ? vol-target.perms.gid : -1;
 if (((uid != -1) || (gid != -1))
- (fchown(fd, vol-target.perms.uid, vol-target.perms.gid)  0)) {
+ (fchown(fd, uid, gid)  0)) {
 virReportSystemError(errno,
  _(cannot chown '%s' to (%u, %u)),
- vol-target.path, vol-target.perms.uid,
- vol-target.perms.gid);
+ vol-target.path, uid, gid);
 goto cleanup;
 }
 if (fchmod(fd, vol-target.perms.mode)  0) {
@@ -356,10 +355,12 @@ virStorageBackendCreateRaw(virConnectPtr conn 
ATTRIBUTE_UNUSED,
 goto cleanup;
 }
 
+uid_t uid = (vol-target.perms.uid == -1) ? getuid() : 
vol-target.perms.uid;
+gid_t gid = (vol-target.perms.gid == -1) ? getgid() : 
vol-target.perms.gid;
+
 if ((createstat = virFileOperation(vol-target.path,
O_RDWR | O_CREAT | O_EXCL | O_DSYNC,
-   vol-target.perms.mode,
-   vol-target.perms.uid, 
vol-target.perms.gid,
+   vol-target.perms.mode, uid, gid,
createRawFileOpHook, hdata,
VIR_FILE_OP_FORCE_PERMS |
(pool-def-type == 
VIR_STORAGE_POOL_NETFS
@@ -491,14 +492,14 @@ cleanup:
 static int virStorageBuildSetUIDHook(void *data) {
 virStorageVolDefPtr vol = data;
 
-if ((vol-target.perms.gid != 0)
+if ((vol-target.perms.gid != -1)
  (setgid(vol-target.perms.gid) != 0)) {
 virReportSystemError(errno,
  _(Cannot set gid to %u before creating %s),
  vol-target.perms.gid, vol-target.path);
 return -1;
 }
-if ((vol-target.perms.uid != 0)
+if ((vol-target.perms.uid != -1)