Re: [libvirt] [PATCH] Fix vm define error with back compat console device
Daniel P. Berrange berra...@redhat.com wrote: On Thu, Jan 15, 2009 at 10:17:56PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: ... + virsh --connect qemu:///session define devs.xml Shouldn't use qemu:///session for test cases like this - this is what the test:///default driver is for, avoiding the fragility danger of using the daemon live hypervisor drivers. There's no failure with test:///default. I'm rather surprised at that - both drivers use identical XML formating routines here, so given the same config, both should fail just as badly. Is this perhaps a case where we need to run it under valgrind to make it reliably fail. Note that virsh itself doesn't fail, in either case. It sends info to libvirtd that causes *it* to segfault, so in order to trigger the bug, I have to run libvirtd. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
Hi David, On Fri, 2009-01-16 at 01:13 +, David Lutterkort wrote: For certain applications, we want libvirt to be able to configure host network interfaces in a variety of ways; currently, we are most interested in teaching libvirt how to set up ordinary ethernet interfaces, bridges, bonding and vlan's. Below is a high-level proposal of how that could be done. Please comment copiously ;) 1. XML Format = The first question is how the user would describe the host interfaces they want. Below are some sketches of what an XML representation of the various kinds of interfaces would look like. This covers the minimal amount of settings for these to be useful, though I am sure we'll need to add more over time. interface device=eth0 onboot=yes hwaddr00:19:d2:9f:88:96/hwaddr dhcp peerdns=yes/ /interface interface device=eth1 onboot=yes hwaddr00:19:d2:9f:88:97/hwaddr static ipaddr=192.168.0.5 gateway=192.168.0.1 netmask=255.255.255.0/ /interface interface device=eth2 onboot=yes hwaddr00:19:d2:9f:88:98/hwaddr /interface You mention MTU below - yes, you'll need that. bond name=bond00 onboot=yes mode=active-backup slave device=eth0 primary=yes/ slave device=eth1/ /bond bridge name=br0 stp=off onboot=yes member device=eth2/ dhcp peerdns=yes/ /bridge I don't think we want to define a bridge here, but more that an interface is shared - i.e. this is a property of eth2. The main concern is that this is the way I'd expect NetworkManager to support it - i.e. that you could configure NetworkManager to share eth0, rather than ask it to create br0 and add eth0 to it. If you just want to create a bridge, you can creati a virtual network. vlan device=eth0 tag=42 reorder_hdr=yes/ I think these last three are also interfaces and should be described with an interface tag e.g. interface device=bond0 onboot=yes bond mode=active-backup/ slave device=eth0 primary=yes/ slave device=eth1/ dhcp/ /interface interface device=eth2 onboot=yes hwaddr00:19:d2:9f:88:98/hwaddr shared bridge=br0/ dhcp/ /interface interface device=eth0 vlan tag=42 reorder_hdr=yes/ /interface All of these should also allow a uuid element for specifying a uuid; I omitted that for brevity. 2. API Changes == There are two options for dealing with network interfaces: (1) use the existing virNetwork* calls or (2) add completely new API calls. Repurposing existing virNetwork* calls -- The existing calls map well to the operations we need for managing interfaces, with a few exceptions: - virNetworkGetAutostart/SetAutostart: depending on how we implement all this (see below), 'autostart' might actually mean 'on boot', not 'when libvirtd starts' - virNetworkGetBridgeName doesn't make sense for interfaces, and should return NULL for interfaces We'll probably also end up adding some functions to query details about an interface, in particular, a call to see what kind of network/interface a virNetworkPtr represents I don't think I like this much - these APIs manage a virtual network which I see as a distinct concept from configuring host hardware. Really, I think keeping the two things separate will actually reduce confusion in the long run. Add completely new virInterface* calls -- This would add roughly the same API calls as the virNetwork* calls, i.e. we'd have something like typedef struct virInterface *virInterfacePtr; int virInterfaceCreate(virInterfacePtr); virInterfacePtr virInterfaceCreateXML(..); ... plus some calls to extract information from a virInterfacePtr This sounds good, but interface is pretty damn generic :-) virNetInterface virNetDevice ... What I don't like about any of these is that I've always imagined we might add further APIs to libvirt for changing a domain's configuration without munging XML e.g. int virDomainGetInterfaces(virDomainPtr domain, virInterfacePtr interfaces, int *ninterfaces); int virInterfaceSetMacAddress(virInterfacePtr interface, const uint8_t mac[6]); So, you can see the potential namespace conflicts ... (But that's pretty irrelevant since I think I'm alone in thinking APIs like that would make sense :-) The second option seems cleaner to me and easier to implement, and avoids any subtle changes in the behavior of existing API, though I don't like that we'll be adding another 20 or so calls to libvirt's public API, with attendant churn both in the drivers and in virsh. I don't think the amount of boilerplate you need to add for new APIs should stop you - the more of this crud we add, the more incentive we'll have to figure out ways to reduce it :-) 3. Implementation = Configuring network interfaces is highly OS and OS-variant/distro
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, 2009-01-16 at 03:29 +, John Levon wrote: On Fri, Jan 16, 2009 at 02:59:17AM +, David Lutterkort wrote: I am not disagreeing with you, but either way, libvirt needs _some_ way to control host interfaces. This is far from obvious to me. Could you explain more? You're right to ask, IMHO. The rationale will be better explained by others, I'm sure, but here goes: Any serious virtualization management tool needs to be able to configure the networking interfaces on a remote host. libvirt aims to be an all-encompassing API for such management tools. The obvious worry is how will we decide where to draw the line? If someone is writing a virt management tool that e.g. needs to install packages on hosts, will we add support for that? Cheers, Mark. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Fine grained Access Control in libVirt
On Fri, Jan 16, 2009 at 12:16:10PM +0900, Atsushi SAKAI wrote: Hi, Dan Would you explain the difference with sVirt? The final goal sVirt seems same form me. (for example, define many security domain etc in .te file.) At this stage sVirt is primarily about protecting guests from each other, and protecting the host from guests. Konrad's suggestions are about protecting guests/hosts from administrators, by providing more fine grained control over what libvirt APIs an admin can invoke on what objects. Both bits of work are required are complementary to each other Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix vm define error with back compat console device
On Fri, Jan 16, 2009 at 09:24:33AM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: On Thu, Jan 15, 2009 at 10:17:56PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: ... + virsh --connect qemu:///session define devs.xml Shouldn't use qemu:///session for test cases like this - this is what the test:///default driver is for, avoiding the fragility danger of using the daemon live hypervisor drivers. There's no failure with test:///default. I'm rather surprised at that - both drivers use identical XML formating routines here, so given the same config, both should fail just as badly. Is this perhaps a case where we need to run it under valgrind to make it reliably fail. Note that virsh itself doesn't fail, in either case. It sends info to libvirtd that causes *it* to segfault, so in order to trigger the bug, I have to run libvirtd. virsh SEGV's nicely enough for me $ ./src/virsh --connect test:///default Welcome to lt-virsh, the virtualization interactive terminal. Type: 'help' for help with commands 'quit' to quit virsh # define a.xml Domain D defined from a.xml virsh # dumpxml D Segmentation fault In fact running virsh at all is redundant - this fails nicely enough if it is just added as a new datafile to the qemuxml2xmltest test case. Index: tests/qemuxml2xmltest.c === RCS file: /data/cvs/libvirt/tests/qemuxml2xmltest.c,v retrieving revision 1.25 diff -u -p -u -p -r1.25 qemuxml2xmltest.c --- tests/qemuxml2xmltest.c 12 Jan 2009 15:09:19 - 1.25 +++ tests/qemuxml2xmltest.c 16 Jan 2009 10:04:07 - @@ -120,6 +120,7 @@ mymain(int argc, char **argv) DO_TEST(serial-many); DO_TEST(parallel-tcp); DO_TEST(console-compat); +DO_TEST(console-overflow); DO_TEST(hostdev-usb-product); DO_TEST(hostdev-usb-address); Index: tests/qemuxml2argvdata/qemuxml2argv-console-overflow.xml === RCS file: tests/qemuxml2argvdata/qemuxml2argv-console-overflow.xml diff -N tests/qemuxml2argvdata/qemuxml2argv-console-overflow.xml --- /dev/null 1 Jan 1970 00:00:00 - +++ tests/qemuxml2argvdata/qemuxml2argv-console-overflow.xml16 Jan 2009 10:04:07 - @@ -0,0 +1,61 @@ +?xml version=1.0? +domain type=kvm + nameD/name + uuidaaa3ae22-fed2-bfbd-ac02-3bea3bcfad82/uuid + memory262144/memory + currentMemory262144/currentMemory + vcpu1/vcpu + os +type arch=i686 machine=pchvm/type +boot dev=cdrom/ + /os + features +acpi/ + /features + clock offset=utc/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu-kvm/emulator +disk type=file device=cdrom + target dev=hdc bus=ide/ + readonly/ +/disk +disk type=file device=floppy + target dev=fdb bus=fdc/ +/disk +disk type=file device=cdrom + target dev=sda bus=scsi/ + readonly/ +/disk +interface type=network + mac address=54:52:00:6c:a0:ca/ + source network=aa/ +/interface +interface type=network + mac address=54:52:00:6c:bb:ca/ + source network=default/ +/interface +serial type=pty/ +serial type=pty/ +serial type=pty/ +parallel type=pty/ +parallel type=pty/ +parallel type=pty/ +input type=mouse bus=ps2/ +sound model=pcspk/ +sound model=es1370/ +hostdev mode=subsystem type=usb + source +address bus=3 device=3/ + /source +/hostdev +hostdev mode=subsystem type=usb + source +vendor id=0x0483/ +product id=0x2016/ + /source +/hostdev + /devices +/domain Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 09:06:05AM +, Mark McLoughlin wrote: On Fri, 2009-01-16 at 03:29 +, John Levon wrote: On Fri, Jan 16, 2009 at 02:59:17AM +, David Lutterkort wrote: I am not disagreeing with you, but either way, libvirt needs _some_ way to control host interfaces. This is far from obvious to me. Could you explain more? You're right to ask, IMHO. The rationale will be better explained by others, I'm sure, but here goes: Any serious virtualization management tool needs to be able to configure the networking interfaces on a remote host. libvirt aims to be an all-encompassing API for such management tools. The obvious worry is how will we decide where to draw the line? If someone is writing a virt management tool that e.g. needs to install packages on hosts, will we add support for that? I think the difference that, is that installing RPMS is not anything todo with virtualization in the general case - it would just be a particular apps approach to working with its OS - as such it would be out of scope for libvirt. Integrating with host networking meanwhile is a fundamental requirement for virtualization for all apps using libvirt, since guests need network connectivity, and thus managing NICs should be within scope. Also note there is existing precedent as the CIM APi, Xen-API, VMWare and VirtualIron APIs all provide for management of host NICs. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 16/25: Fix GNULIB warnings on Mingw
On Fri, Jan 16, 2009 at 12:14:51PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: The GNULIB Makefile.am doesn't set any particular compiler warning flags in the belief that upstream GNULIB developers sort out all warnings before committing their code. This is reasonable for builds on mainstream Linux platforms, but MinGW builds of GNULIB don't have nearly the same level of testing. So This patch adds the libvirt $(WARN_CFLAGS) to the gnulib Makefile.am and then fixes the problems this showed up. Nothing of consequence really. Leave it upto gnulib developers whether its worth fixing these. Makefile.am|2 gettimeofday.c |5 +- ioctl.c|1 poll.c |4 - strerror.c | 128 - I've dealt with the poll.c problems upstream. 1) Your patch to remove unused var decls 2) added a pragma to avoid the warning about the unsigned_var 0 tests All of this looks fine to commit here. I've posted an adjusted strerror.c change upstream, without so many casts. Dan, what warning does the ioctl one fix? Same for gettimeofday. Here's the full set of warnings I got gettimeofday.c: In function 'rpl_localtime': gettimeofday.c:54: warning: nested extern declaration of 'localtime' ioctl.c:32: warning: no previous prototype for 'rpl_ioctl' poll.c: In function 'rpl_poll': poll.c:411: warning: comparison of unsigned expression 0 is always false poll.c:429: warning: unused variable 'optlen' strerror.c: In function 'rpl_strerror': strerror.c:48: warning: return discards qualifiers from pointer target type strerror.c:54: warning: return discards qualifiers from pointer target type strerror.c:56: warning: return discards qualifiers from pointer target type strerror.c:58: warning: return discards qualifiers from pointer target type strerror.c:60: warning: return discards qualifiers from pointer target type [snip more strerror] Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 17/25: Concurrent client dispatch in libvirtd
On Tue, Jan 13, 2009 at 09:51:00PM +, Daniel P. Berrange wrote:
[...]
From a peephole view of the patch, it looks sane enough, and I think
we should commit it and debug any fallout later. One tiny comment:
+struct qemud_client_message;
+
+struct qemud_client_message {
+char buffer [REMOTE_MESSAGE_MAX + REMOTE_MESSAGE_HEADER_XDR_LEN];
+unsigned int bufferLength;
+unsigned int bufferOffset;
+
+int async : 1;
+
+struct qemud_client_message *next;
+};
(1) You don't need to predeclare the struct. (2) This means the
struct is always the same size as the maximum-sized message.
Shouldn't we malloc the buffer?
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix vm define error with back compat console device
On Fri, Jan 16, 2009 at 12:42:35PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: On Fri, Jan 16, 2009 at 09:24:33AM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: On Thu, Jan 15, 2009 at 10:17:56PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: ... + virsh --connect qemu:///session define devs.xml Shouldn't use qemu:///session for test cases like this - this is what the test:///default driver is for, avoiding the fragility danger of using the daemon live hypervisor drivers. There's no failure with test:///default. I'm rather surprised at that - both drivers use identical XML formating routines here, so given the same config, both should fail just as badly. Is this perhaps a case where we need to run it under valgrind to make it reliably fail. Note that virsh itself doesn't fail, in either case. It sends info to libvirtd that causes *it* to segfault, so in order to trigger the bug, I have to run libvirtd. virsh SEGV's nicely enough for me $ ./src/virsh --connect test:///default Welcome to lt-virsh, the virtualization interactive terminal. Type: 'help' for help with commands 'quit' to quit virsh # define a.xml Domain D defined from a.xml virsh # dumpxml D Segmentation fault Ah. I'd only run the define, which is enough to make libvirtd fail when using qemu:///session. Oh, that'll be because when you run 'define' in QEMU, it parses the XML and then runs dumpxml internally to save it out to disk In fact running virsh at all is redundant - this fails nicely enough if it is just added as a new datafile to the qemuxml2xmltest test case. Putting that test closer to the target code is good, so ACK, assuming it passes make distcheck. However, part of my goal in running the qemu:///session server is to get more coverage on the server side. make check does very little testing of libvirtd, currently. So if you don't mind, I'll also add a comparable test using qemu:///session and unix_sock_dir, so it doesn't interfere with existing state. I think this is the wrong way to go about testing the real live hypervisor drivers. For those we should have something that is like the libvirt-CIM test suite. eg a test system which has code to run all the libvirt APIs. This single test system is then run in a controlled environment once for each hypervisor URI that we support. This will mean we exercise all the core codepaths for the real drivers, and also validate that they are all responding in the same way with consistent semantics / underling operations. Adding ad-hoc test cases to leverage qemu:///session is wasted effort because its creating a functional/integration test suite which is neccessarily tied to QEMU driver, as opposed to being a generic solution. The 'make check' testsuite should be focused on unit testing either by calling specific APIs relating to the hypervisor specific code, or be running commands with the 'test' driver. The test driver has sufficient functionality to let us functionally test core operation of the libvirtd daemon too. So, this is the distinction between calling 'qemudBuildCommandLine' to test QEMU driver command line arg processing, and running the 'start' operation on the QEMU driver which indirectly calls the qemudBuildCommandLine method. Maybe you want to keep the extra disk and interface sections after all? (the ones that you suggested I remove) Better coverage? There's already a tonne of data files in qemuxml2argvdata that exercise the disk/nic/hostdev bits of the XML parser. The XML parsing is the one area where we have generally good test coverage for all common paths and just need to look at the code coverage reports to find edge cases like this scenario we're fixing here. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Fine grained Access Control in libVirt
Daniel summarized my approach nicely. Basically I'm looking at enabling multi-tenancy administration where several admins can exist but they can only see and/or manipulate with resources (VMs, storage, networks) assigned to them. By making use of a generic AC-module approach where actions gets passed through arbitrary complex access control can be enforced since the AC-module could implement/interface different schemes of granting/denying access depending on what enforcing policy wants to be used. One could for example use SELinux as a scheme to enable RBAC and/or tie it together with policies for sVirt. An initial implementation step would be realizing the AC-module foundation and starting with moving out the RW/RO enforcement (currently residing within libvirt.c) as first basic enforcement scheme. Freundliche Grüsse / Best regards Konrad Eriksson Research Software Engineer Trusted Computing / Security Assurance Email: k...@zurich.ibm.com Phone: +41 (0)44 724 84 28 IBM Zurich Research Laboratory Saeumerstrasse 4 8803 Rueschlikon Switzerland From: Daniel P. Berrange berra...@redhat.com To: Atsushi SAKAI sak...@jp.fujitsu.com Cc: Konrad Eriksson1 k...@zurich.ibm.com, libvir-list@redhat.com Date: 01/16/2009 10:57 AM Subject: Re: [libvirt] Fine grained Access Control in libVirt On Fri, Jan 16, 2009 at 12:16:10PM +0900, Atsushi SAKAI wrote: Hi, Dan Would you explain the difference with sVirt? The final goal sVirt seems same form me. (for example, define many security domain etc in .te file.) At this stage sVirt is primarily about protecting guests from each other, and protecting the host from guests. Konrad's suggestions are about protecting guests/hosts from administrators, by providing more fine grained control over what libvirt APIs an admin can invoke on what objects. Both bits of work are required are complementary to each other Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| image/gif smime.p7s Description: S/MIME Cryptographic Signature -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 18/25: Dynamic thread workers pool
On Tue, Jan 13, 2009 at 05:46:08PM +, Daniel P. Berrange wrote:
@@ -1948,6 +2000,26 @@ static int qemudRunLoop(struct qemud_ser
}
}
+/* If number of active workers exceeds both the min_workers
+ * threshold and the number of clients, then kill some
+ * off */
+for (i = 0 ; (i server-nworkers
+ server-nactiveworkers server-nclients
+ server-nactiveworkers min_workers) ; i++) {
+
+if (server-workers[i].active
+!server-workers[i].processing) {
+server-workers[i].quit = 1;
+
+virCondBroadcast(server-job);
+virMutexUnlock(server-lock);
+pthread_join(server-workers[i].thread, NULL);
+virMutexLock(server-lock);
+server-workers[i].active = 0;
+server-nactiveworkers--;
+}
+}
+
Doesn't this cause the main loop to hang -- eg. if we happen to try to
kill of a worker which is doing some lengthy operation?
+struct qemud_worker {
+pthread_t thread;
+int active :1;
+int processing :1;
+int quit : 1;
I guess maybe I'm unclear about the meaning of these flags. What's
the difference between active processing?
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
Read my OCaml programming blog: http://camltastic.blogspot.com/
Fedora now supports 68 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 8/25: Concurrent dispatch of RPC methods
On Fri, Jan 16, 2009 at 12:49:06PM +0100, Jim Meyering wrote:
Daniel P. Berrange berra...@redhat.com wrote:
This patch re-writes the code for dispatching RPC calls in the
remote driver to allow use from multiple threads. Only one thread
is allowed to send/recv on the socket at a time though. If another
thread comes along it will put itself on a queue and go to sleep.
The first thread may actually get around to transmitting the 2nd
thread's request while it is waiting for its own reply. It may
even get the 2nd threads reply, if its own RPC call is being really
slow. So when a thread wakes up from sleeping, it has to check
whether its own RPC call has already been processed. Likewise when
a thread owning the socket finishes with its own wor, it may have
to pass the buck to another thread. The upshot of this, is that
we have mutliple RPC calls executing in parallel, and requests+reply
are no longer guarenteed to be FIFO on the wire if talking to a new
enough server.
This refactoring required use of a self-pipe/poll trick for sync
between threads, but fortunately gnulib now provides this on Windows
too, so there's no compatability problem there.
Quick summary: dense ;-)
though lots of moved code.
I haven't finished, but did find at least one problem, below.
diff --git a/src/remote_internal.c b/src/remote_internal.c
...
@@ -114,6 +164,11 @@ struct private_data {
virDomainEventQueuePtr domainEvents;
/* Timer for flushing domainEvents queue */
int eventFlushTimer;
+
+/* List of threads currently doing dispatch */
+int wakeupSend;
+int wakeupRead;
How about appending FD to indicate these are file descriptors.
The names combined with the comment (which must apply to waitDispatch)
made me wonder what they represented. Only when I saw them used
in safewrite /saferead calls did I get it.
Yes, good idea - and its not really a list of threads either,
so the comment is a little misleading :-)
+/* Serialise header followed by args. */
+xdrmem_create (xdr, rv-buffer+4, REMOTE_MESSAGE_MAX, XDR_ENCODE);
+if (!xdr_remote_message_header (xdr, hdr)) {
+error (flags REMOTE_CALL_IN_OPEN ? NULL : conn,
+ VIR_ERR_RPC, _(xdr_remote_message_header failed));
+goto error;
+}
+
+if (!(*args_filter) (xdr, args)) {
+error (flags REMOTE_CALL_IN_OPEN ? NULL : conn, VIR_ERR_RPC,
+ _(marshalling args));
+goto error;
+}
+
+/* Get the length stored in buffer. */
+rv-bufferLength = xdr_getpos (xdr);
+xdr_destroy (xdr);
+
+/* Length must include the length word itself (always encoded in
+ * 4 bytes as per RFC 4506).
+ */
+rv-bufferLength += 4;
+
+/* Encode the length word. */
+xdrmem_create (xdr, rv-buffer, 4, XDR_ENCODE);
+if (!xdr_int (xdr, (int *)rv-bufferLength)) {
+error (flags REMOTE_CALL_IN_OPEN ? NULL : conn, VIR_ERR_RPC,
+ _(xdr_int (length word)));
I haven't done enough xdr* work to know, and man pages
didn't provide an immediate answer:
Is there no need to call xdr_destroy on this error path?
I'd expect xdrmem_create to do any allocation, not xdr_int.
There are many like this.
Yes, the 'error:' codepath should be calling 'xdr_destroy(xdr)'
to ensure free'ing of memory.
+goto error;
+}
+xdr_destroy (xdr);
+
+return rv;
+
+error:
+VIR_FREE(ret);
+return NULL;
The above should free rv, not ret:
VIR_FREE(rv);
Doh, bad naming convention for arguments - we always use 'ret' for return
values. I should rename the argument to 'reply' since its what contains
the RPC reply, so we can use the normal convention of 'ret' for return
value.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 17/25: Concurrent client dispatch in libvirtd
On Fri, Jan 16, 2009 at 12:01:16PM +, Richard W.M. Jones wrote:
On Tue, Jan 13, 2009 at 09:51:00PM +, Daniel P. Berrange wrote:
[...]
From a peephole view of the patch, it looks sane enough, and I think
we should commit it and debug any fallout later. One tiny comment:
+struct qemud_client_message;
+
+struct qemud_client_message {
+char buffer [REMOTE_MESSAGE_MAX + REMOTE_MESSAGE_HEADER_XDR_LEN];
+unsigned int bufferLength;
+unsigned int bufferOffset;
+
+int async : 1;
+
+struct qemud_client_message *next;
+};
(1) You don't need to predeclare the struct. (2) This means the
struct is always the same size as the maximum-sized message.
Shouldn't we malloc the buffer?
I did consider whether we should malloc the buffer, but this only
helps when receiving the RPC request. We know there what the size
of the incoming message is, from the length field in the header.
When we are generating the reply message though, we don't know how
big it'll be until we've made the XDR calls to serialize the struct
into the XDR. So we'd need to have the full sized buffer for filling
out the reply.
To avoid having to malloc() a 'struct qemud_client_message' for the
RPC reply, I currently just re-use the existing malloc()'d one we
had for the RPC request, since that's now unused. If we only malloc'd
the buffer that we needed for the request, then we'd need to realloc
it to the full size for the reply anyway.
So I think just declaring the struct to the full buffer size is an
acceptable approach - particularly since we never have any of these
structs on the stack - they're all on the heap.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 18/25: Dynamic thread workers pool
On Fri, Jan 16, 2009 at 12:10:35PM +, Richard W.M. Jones wrote:
On Tue, Jan 13, 2009 at 05:46:08PM +, Daniel P. Berrange wrote:
@@ -1948,6 +2000,26 @@ static int qemudRunLoop(struct qemud_ser
}
}
+/* If number of active workers exceeds both the min_workers
+ * threshold and the number of clients, then kill some
+ * off */
+for (i = 0 ; (i server-nworkers
+ server-nactiveworkers server-nclients
+ server-nactiveworkers min_workers) ; i++) {
+
+if (server-workers[i].active
+!server-workers[i].processing) {
+server-workers[i].quit = 1;
+
+virCondBroadcast(server-job);
+virMutexUnlock(server-lock);
+pthread_join(server-workers[i].thread, NULL);
+virMutexLock(server-lock);
+server-workers[i].active = 0;
+server-nactiveworkers--;
+}
+}
+
Doesn't this cause the main loop to hang -- eg. if we happen to try to
kill of a worker which is doing some lengthy operation?
If the worker is currently processing an RPC call, its 'processing'
flag will be set, and thus we won't consider it a candidate for
killing off.
+struct qemud_worker {
+pthread_t thread;
+int active :1;
+int processing :1;
+int quit : 1;
I guess maybe I'm unclear about the meaning of these flags. What's
the difference between active processing?
I'll add comments to this struct before commiting. Their meanings
are:
active: the thread actually exists
processing: the thread is currently processing an RPC call
quit: the thread has been asked to quit
At startup we malloc enough 'struct qemud_worker' objects to hold the
entire 'max_workers' set, but we only start threads for 'min_workers.
This is what the 'active' field tracks. The 'processing' flag is there
so that when we want to kill off a thread, we won't block waiting for
a thread that's currently busy working. And the 'quit' flag is that
condition that a thread checks upon waking up from its condition variable
sleep to see if it was asked to quit.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 20/25: Import gnulbi random_r module
On Tue, Jan 13, 2009 at 05:47:03PM +, Daniel P. Berrange wrote:
Import the gnulib 'random_r' module, which provides a nice
strong random number generator that is portable across OS.
bootstrap|1
gnulib/lib/.cvsignore| 12 -
gnulib/lib/Makefile.am | 13 +
gnulib/lib/gettimeofday.c|5
gnulib/lib/ioctl.c |1
gnulib/lib/poll.c|4
gnulib/lib/random_r.c| 420
+++
gnulib/lib/strerror.c| 128 ++---
diff --git a/gnulib/lib/strerror.c b/gnulib/lib/strerror.c
--- a/gnulib/lib/strerror.c
+++ b/gnulib/lib/strerror.c
@@ -45,89 +45,89 @@ rpl_strerror (int n)
{
# if GNULIB_defined_ETXTBSY
case ETXTBSY:
- return (char*)Text file busy;
+ return Text file busy;
# endif
# if GNULIB_defined_ESOCK /* native Windows platforms */
/* EWOULDBLOCK is the same as EAGAIN. */
case EINPROGRESS:
- return (char*)Operation now in progress;
+ return Operation now in progress;
Reverts an earlier change?
Rich.
--
Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones
virt-top is 'top' for virtual machines. Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://et.redhat.com/~rjones/virt-top
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 21/25: Use random_r for random numbers
On Tue, Jan 13, 2009 at 05:47:27PM +, Daniel P. Berrange wrote: Now that gnulib's rand module is imported, we have a decent quality random number generator that's portable. We don't want to mess with the apps state, so in virInitialize() we explicitly initialize our own private random nubmer generator state with virRandomInitialize(). The util.h file gains a convenience macro, since random_r() is horrible to call and we need to protect our global state int virRandom(int max) +1. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 22/25: Make configure output aligned
On Tue, Jan 13, 2009 at 05:47:49PM +, Daniel P. Berrange wrote: The configure output looked untidy, so this aligns things correctly. configure.in | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) Cleanup always good. +1 Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 19/25: Remove use of non-threadsafe POSIX apis
On Fri, Jan 16, 2009 at 12:21:59PM +, Richard W.M. Jones wrote:
On Tue, Jan 13, 2009 at 05:46:37PM +, Daniel P. Berrange wrote:
+char buf[1024];
sysconf (_SC_GETPW_R_SIZE_MAX)?
Looking at glibc's implementation of getpwuid (which uses getpwuid_r),
I see that glibc dynamically reallocates the buffer as necessary to
the correct size for the return value. The logic of this is fairly
simple so maybe we should do the same?
From glibc:
buffer = malloc (/*some_initial_size*/);
while (buffer != NULL
(getpwuid_r (const char *name, resbuf, buffer,
buffer_size, result)
== ERANGE))
{
char *new_buf;
buffer_size *= 2;
new_buf = (char *) realloc (buffer, buffer_size);
if (new_buf == NULL)
{
free (buffer);
errno = ENOMEM;
}
buffer = new_buf;
}
Anyhow, +1 but I'd be happier if these functions were centralized in
somewhere like src/utils.c.
That's a good idea - in all the cases where we currently use getpwuid
all we actually want is the home directory path. So we could add a
simple func:
char *virUserHomeDirectory(uid_t uid);
and hide all the horrific code in there.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 23/25: Add domain events to test driver
On Tue, Jan 13, 2009 at 09:51:52PM +, Daniel P. Berrange wrote: On Tue, Jan 13, 2009 at 05:48:12PM +, Daniel P. Berrange wrote: This adds support for the domain events in the test driver. Code is following the same pattern as the impl in the QEMU driver. test.c | 223 +++-- 1 file changed, 217 insertions(+), 6 deletions(-) Updated patch to not queue events if no event loop impl is defined (as is common from virsh). This is what was breaking the tests cases Jim seems to have covered this ... Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 24/25: Update python error reporting
On Tue, Jan 13, 2009 at 05:48:40PM +, Daniel P. Berrange wrote: The virGetLastError() method is now stored in a thread local and virConnGetLastError() is deprecated, so switch to only use the former. Also, no need to copy the error object since it is thread local already thus guarenteed not to change behind our backs libvir.c | 44 ++-- libvir.py |7 +++ 2 files changed, 25 insertions(+), 26 deletions(-) Simple, +1. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 25/25: Support ac97 sound card
On Wed, Jan 14, 2009 at 11:41:32AM +, Daniel P. Berrange wrote: On Tue, Jan 13, 2009 at 05:49:02PM +, Daniel P. Berrange wrote: QEMU now has support for a sound card of type ac97, so enable that in the XML parser / qemu driver. Also remove some unused cruft relating to sound in Xen. domain_conf.c |3 ++- domain_conf.h |1 + xend_internal.c | 46 -- 3 files changed, 3 insertions(+), 47 deletions(-) Now we added 'ac97', we can no longer blindly convert Xen's 'all' string into all possible sound card types - must restrict it to just the 3 that were around historically. This fixes the Xen test case Cole Jim seem to have covered this one. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my OCaml programming blog: http://camltastic.blogspot.com/ Fedora now supports 68 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] sharing multiple identical USB devices?
On Thu, Jan 15, 2009 at 04:41:00PM -0500, Dr. Michael J. Chudobiak wrote: Hi, I have Windows XP running on Fedora 10. I can successfully share a single USB-to-RS232 converter by inserting this in my xml file: I'm assuming this must be with qemu or KVM. What qemu command line does this generate? (ie. ps ax | grep qemu) Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 11/25: Public APIs for incrementing refcount
On Thu, Jan 15, 2009 at 05:18:40PM +, Richard W.M. Jones wrote: On Tue, Jan 13, 2009 at 05:43:08PM +, Daniel P. Berrange wrote: With the domain events code, the callbacks triggered upon events get given a virDomainPtr object instance. In many cases it'd be desirable to grab this object and keep it in your app code. Unfortunately it is free'd the moment the callback finishes executing. When allowing multiple threads to access a single virConnectPtr object it is neccessary to ensure no thread releases it (virConnectCLose) while another thread is still using it. The way to address both of these problems is to allow an application to take an explicit reference on the object in question. So this patch exposes methods to allow an app to increment the ref count on all our public objects. To release the ref count, the existing virConectClose/ virDOmainFree, etc methods suffice include/libvirt/libvirt.h|6 + include/libvirt/libvirt.h.in |6 + src/libvirt.c| 183 +++ src/libvirt_public.syms | 12 ++ 4 files changed, 206 insertions(+), 1 deletion(-) Poor man's garbage collection ... +1. Yeah it's unfortunate we have to add this at the API level, but unfortunately there is no way around. I wonder if we could resurrect some of the deprecated fields of error structure now, seems to me that would be possible now. Patch looks fine, +1 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] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 01:13:18AM +, David Lutterkort wrote: For certain applications, we want libvirt to be able to configure host network interfaces in a variety of ways; currently, we are most interested in teaching libvirt how to set up ordinary ethernet interfaces, bridges, bonding and vlan's. [...] 3. Implementation = Configuring network interfaces is highly OS and OS-variant/distro dependant. There are at least two different ways how libvirt can go about modifying the host to create interfaces: 1. Modify the system's network setup scripts (ifcfg-XXX on RH) 2. Directly use the system's network utilities like ifconfig 3. Rely on NetworkManager (not an option right now, as NM doesn't know about bridges and the like) I must be missing something here ... how can libvirt modify stuff inside the guest? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 17/25: Concurrent client dispatch in libvirtd
On Fri, Jan 16, 2009 at 12:15:42PM +, Daniel P. Berrange wrote: When we are generating the reply message though, we don't know how big it'll be until we've made the XDR calls to serialize the struct into the XDR. So we'd need to have the full sized buffer for filling out the reply. Fair enough. PortableXDR has been taking liberties with the 'traditional' xdr*_create backends, and one that I mean to implement is a variant of xdrmem which dynamically allocates the buffer. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 12:35:55PM +, Richard W.M. Jones wrote: On Fri, Jan 16, 2009 at 01:13:18AM +, David Lutterkort wrote: For certain applications, we want libvirt to be able to configure host network interfaces in a variety of ways; currently, we are most interested in teaching libvirt how to set up ordinary ethernet interfaces, bridges, bonding and vlan's. [...] 3. Implementation = Configuring network interfaces is highly OS and OS-variant/distro dependant. There are at least two different ways how libvirt can go about modifying the host to create interfaces: 1. Modify the system's network setup scripts (ifcfg-XXX on RH) 2. Directly use the system's network utilities like ifconfig 3. Rely on NetworkManager (not an option right now, as NM doesn't know about bridges and the like) I must be missing something here ... how can libvirt modify stuff inside the guest? It can't - this is all about configuring managing networking on the host OS. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 12:41:42PM +, Daniel P. Berrange wrote: On Fri, Jan 16, 2009 at 12:35:55PM +, Richard W.M. Jones wrote: On Fri, Jan 16, 2009 at 01:13:18AM +, David Lutterkort wrote: For certain applications, we want libvirt to be able to configure host network interfaces in a variety of ways; currently, we are most interested in teaching libvirt how to set up ordinary ethernet interfaces, bridges, bonding and vlan's. [...] 3. Implementation = Configuring network interfaces is highly OS and OS-variant/distro dependant. There are at least two different ways how libvirt can go about modifying the host to create interfaces: 1. Modify the system's network setup scripts (ifcfg-XXX on RH) 2. Directly use the system's network utilities like ifconfig 3. Rely on NetworkManager (not an option right now, as NM doesn't know about bridges and the like) I must be missing something here ... how can libvirt modify stuff inside the guest? It can't - this is all about configuring managing networking on the host OS. Well OK then, how does the information get passed into the guest? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 12:44:21PM +, Richard W.M. Jones wrote: On Fri, Jan 16, 2009 at 12:41:42PM +, Daniel P. Berrange wrote: On Fri, Jan 16, 2009 at 12:35:55PM +, Richard W.M. Jones wrote: On Fri, Jan 16, 2009 at 01:13:18AM +, David Lutterkort wrote: For certain applications, we want libvirt to be able to configure host network interfaces in a variety of ways; currently, we are most interested in teaching libvirt how to set up ordinary ethernet interfaces, bridges, bonding and vlan's. [...] 3. Implementation = Configuring network interfaces is highly OS and OS-variant/distro dependant. There are at least two different ways how libvirt can go about modifying the host to create interfaces: 1. Modify the system's network setup scripts (ifcfg-XXX on RH) 2. Directly use the system's network utilities like ifconfig 3. Rely on NetworkManager (not an option right now, as NM doesn't know about bridges and the like) I must be missing something here ... how can libvirt modify stuff inside the guest? It can't - this is all about configuring managing networking on the host OS. Well OK then, how does the information get passed into the guest? There isn't anything to pass into the guest . This is about how to configure host network interfaces for bridging, bonding, vlans, etc eg, So you can take an host with a NIC, eth0, put that NIC into a bridge, br0, and thus now have the ability to create guests using a bridged networking configuration. As it currently stands admins have to manually configure bridges in their host by editing /etc/sysconfig/networking-scripts/ifcfg-eth0 This proposal is to provide an API for doing this in the host instead of requiring that admin do it manually ahead of time. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Solaris least privilege support
On Fri, Jan 16, 2009 at 08:35:16AM +0100, Daniel Veillard wrote: The comment and the code don't seems to match, and it seems to me Oops, will fix the comment. that this code would fail except in the first time the daemon is launched because mkdir /var/run/libvirt will return -1 and errno EEXIST in all following cases. I'm worried about this, What do you mean? /var/run is a tmpfs on Solaris so must mkdir after every reboot. regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Solaris least privilege support
On Thu, Jan 15, 2009 at 09:19:39AM -0800, john.le...@sun.com wrote:
[..snip..]
+/* Change the group ownership of /var/run/libvirt to unix_sock_gid */
+if (geteuid () == 0) {
+const char *rundir = LOCAL_STATE_DIR /run/libvirt;
+
+if (mkdir (rundir, 0755)) {
virFileMakePath?
-- Guido
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 15/25: Prohibit non-threadsafe POSIX apis
On Tue, Jan 13, 2009 at 05:44:51PM +, Daniel P. Berrange wrote: The readdir one is also unneccessary, since reading from a single DIR* is safe from a single thread. readdir_r is also horrific http://womble.decadentplace.org.uk/readdir_r-advisory.html That web page also says that readdir isn't thread safe, in that an implementation could still use some global state (outside of the DIR*): quote Programs using readdir_r may be able to use readdir. According to POSIX the buffer readdir uses is not shared between directory streams. However readdir is not guaranteed to be thread-safe and some implementations may use global state, so for portability the use of readdir in a multithreaded program should be controlled using a mutex. /quote Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Solaris least privilege support
On Fri, Jan 16, 2009 at 02:03:39PM +0100, Guido G?nther wrote: virFileMakePath? The implementation of this is insufficient, and unnecessary. regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] patch queue: bridge-script
On Fri, Jan 16, 2009 at 12:31:24PM +, Richard W.M. Jones wrote: # Node ID 04139c088854c23dc548c0f9f4abf54c4ed07e0d # Parent 253da3acb103a4ce764cb8192a65a4d51833c2f1 patch queue: bridge-script Hi John, is there some explanatory text for this patch? Erk, sorry, I accidentally deleted the patch comment. Previously, libvirt allowed the network bridge configuration to specify a script. For back-compatibility, we must continue to allow this. Additionally, on Solaris, vif-bridge doesn't exist, so if we see a Xen config using 'vif-vnic', assume it's the bridge type, even if a bridge isn't explicitly given. This also fixes a leak if ipaddr was in the config, and a bridge was specified. regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 09:00:16AM +, Mark McLoughlin wrote: What I don't like about any of these is that I've always imagined we might add further APIs to libvirt for changing a domain's configuration without munging XML e.g. I'm glad to hear this is being considered: you're most certainly not alone. This is a significant wart in the current API IMHO. regards john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix vm define error with back compat console device
Daniel P. Berrange berra...@redhat.com wrote: On Fri, Jan 16, 2009 at 12:42:35PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: On Fri, Jan 16, 2009 at 09:24:33AM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: On Thu, Jan 15, 2009 at 10:17:56PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: ... + virsh --connect qemu:///session define devs.xml Shouldn't use qemu:///session for test cases like this - this is what the test:///default driver is for, avoiding the fragility danger of using the daemon live hypervisor drivers. There's no failure with test:///default. I'm rather surprised at that - both drivers use identical XML formating routines here, so given the same config, both should fail just as badly. Is this perhaps a case where we need to run it under valgrind to make it reliably fail. Note that virsh itself doesn't fail, in either case. It sends info to libvirtd that causes *it* to segfault, so in order to trigger the bug, I have to run libvirtd. virsh SEGV's nicely enough for me $ ./src/virsh --connect test:///default Welcome to lt-virsh, the virtualization interactive terminal. Type: 'help' for help with commands 'quit' to quit virsh # define a.xml Domain D defined from a.xml virsh # dumpxml D Segmentation fault Ah. I'd only run the define, which is enough to make libvirtd fail when using qemu:///session. Oh, that'll be because when you run 'define' in QEMU, it parses the XML and then runs dumpxml internally to save it out to disk Exactly. It exercises a different code path that also terminates in the code fixed by Cole's patch. In fact running virsh at all is redundant - this fails nicely enough if it is just added as a new datafile to the qemuxml2xmltest test case. Putting that test closer to the target code is good, so ACK, assuming it passes make distcheck. However, part of my goal in running the qemu:///session server is to get more coverage on the server side. make check does very little testing of libvirtd, currently. So if you don't mind, I'll also add a comparable test using qemu:///session and unix_sock_dir, so it doesn't interfere with existing state. I think this is the wrong way to go about testing the real live hypervisor drivers. For those we should have something that is like the libvirt-CIM test suite. eg a test system which has code to run all the libvirt APIs. This single test system is then run ?? I too would like to have it all now. I'm proposing what seems to me to be an easy way to incrementally add test cases like the one to exercise Cole's bug. We need to do much more of this (adding tests with each bug fix), not less. in a controlled environment once for each hypervisor URI that we support. This will mean we exercise all the core codepaths for the real drivers, and also validate that they are all responding in the same way with consistent semantics / underling operations. Adding ad-hoc test cases to leverage qemu:///session is wasted effort because its creating a functional/integration test suite which is neccessarily tied to QEMU driver, as opposed to being a generic solution. IMHO it's not wasted at all. This is a test that can be run by non-root users, and getting *some* coverage without requiring sudo or root is essential. However, the test can obviously be improved to cover more drivers. I propose to add the test now (new version below), and eventually put a loop around things so that, it does something like this for each $driver: * if running as root and compiled with $driver support, then run the test using $driver The 'make check' testsuite should be focused on unit testing either by calling specific APIs relating to the hypervisor specific code, If you don't want any more integration tests like this to be run via make check, I can put them on a different target. Having a good mix of tests can only help. IME, both types of tests are necessary. or be running commands with the 'test' driver. The test driver has sufficient functionality to let us functionally test core operation of the libvirtd daemon too. Some, but obviously not all. So, this is the distinction between calling 'qemudBuildCommandLine' to test QEMU driver command line arg processing, and running the 'start' operation on the QEMU driver which indirectly calls the qemudBuildCommandLine method. Maybe you want to keep the extra disk and interface sections after all? (the ones that you suggested I remove) Better coverage? There's already a tonne of data files in qemuxml2argvdata that exercise the disk/nic/hostdev bits of the XML parser. The XML parsing is the one area where we have generally good test coverage for all common paths and just need to look at the code coverage reports to find edge cases like this scenario we're fixing here. Either way is
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 01:24:53PM +, John Levon wrote: On Fri, Jan 16, 2009 at 10:11:02AM +, Daniel P. Berrange wrote: Integrating with host networking meanwhile is a fundamental requirement for virtualization for all apps using libvirt, since guests need network connectivity, and thus managing NICs should be within scope. I don't think that's much of an argument. Plenty of things can be considered fundamental. My kernel version certainly is, so why isn't libvirt letting me upgrade that? What about my firewall? Why isn't libvirt configuring my iSCSI target for me? The kernel version isn't fundamental to the task of provisioning and configuring a guest VM. When deploying a VM there is no general requirement to upgrade the host kernel. When deploying a VM there very much is a requirement to configure physical resources in the host such as storage, and networking. We should be considering why libvirt is /well-placed/ to configure the host. I think it should be pretty clear that it's actually not: the problems around distro differences alone is a good indication. The proposed API is anaemic enough to not be of much use. The existance of many different impls is exactly the reason for libvirt to have this capability. Libvirt is providing a consistent mgmt API for management of guests and host networking interfaces is as much a part of this as the storage management. Libvirt is providing this capability across virtualization technology. If we did not include the NIC mgmt then apps using libvirt would not only need to write different code for each OS, but also for Xen, VMWare, etc each of have their own APIs for network management which is just not helpful for apps using libvirt. We already have this problem in apps using libvirt needing to cope with different OS networking setups and thus duplicating all this horrible code in every user of libvirt, which is why networking mgmt is a important goal within scope. This is way beyond carving out the physical system into virtual chunks and it's a big step towards lib*virt* becoming libmanagement. Not really - it is precisely about carving up / managing physical resources that are related to the guest configuration. It is not about arbitrary OS management - we don't want an API to deploy RPMs, or configure Apache, or any other things related to general OS management. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix vm define error with back compat console device
Jim Meyering j...@meyering.net wrote: ... FYI, here's a new version of this latest test. Now it uses the new unix_sock_dir setting to avoid risk of interfering with any existing qemu-based settings. (this patch depends on the unix_sock_dir-adding patch that's still waiting for an ACK. Also, I have two or three other test-adding patches that depend on unix_sock_dir. From 0558cb2d432e44213ecd5b2b95dee8eac1f31276 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Thu, 15 Jan 2009 19:51:39 +0100 Subject: [PATCH] exercise a bug that could make libvirtd segfault * tests/define-dev-segfault: New file. * tests/Makefile.am (test_scripts): Add define-dev-segfault. --- tests/Makefile.am |1 + tests/define-dev-segfault | 69 + 2 files changed, 70 insertions(+), 0 deletions(-) create mode 100755 tests/define-dev-segfault diff --git a/tests/Makefile.am b/tests/Makefile.am index c735d62..39b5727 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -60,6 +60,7 @@ if WITH_LIBVIRTD test_scripts += \ cpuset \ daemon-conf \ + define-dev-segfault \ int-overflow\ libvirtd-fail \ libvirtd-pool \ [in case you try to apply this patch, ... ] Note that this hunk will not apply for you, since it depends on an earlier still-pending one that aligned the backslashes. The point is just to add the new test name to the list. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 09:00:16AM +, Mark McLoughlin wrote: Hi David, On Fri, 2009-01-16 at 01:13 +, David Lutterkort wrote: 3. Implementation = Configuring network interfaces is highly OS and OS-variant/distro dependant. There are at least two different ways how libvirt can go about modifying the host to create interfaces: 1. Modify the system's network setup scripts (ifcfg-XXX on RH) 2. Directly use the system's network utilities like ifconfig 3. Rely on NetworkManager (not an option right now, as NM doesn't know about bridges and the like) It has to be an option - even if it only supports a subset of what the other options support. I really wouldn't like to see us push ahead with this until we have a plan for how this will work with NetworkManager (and a rough agreement for how future support for bonding etc. in NetworkManager might be configured) Absolutely - NetworkManager is a fundamental requirement for this to be viable, since it is the default mgmt backend for Linux desktop deployments, and potentially even for Server deployments in future. Also need to take into account the network mgmt capabilities of other virt APIs such as VMWare / XenAPI, since it is very desirable to be able to target these as a backend impl when using those hypervisor drivers Option (1) saves us from replicating every bit of network setup functionality that those scripts already have - besides configuring the interface, we also might have to setup routes, run dhclient etc. The important benefit of (1), is that libvirt plays nicely with distro scripts, rather than fighting/racing with them. Option (2) would require far fewer backend implementations than (1) - we should be able to get away with one implementation for Linux, rather than one for Fedora/RHEL, one for Debian, one for SuSe, three for gentoo etc. The thing is, this has to integrate with existing configuration - there's no point in futzing about with ip link set eth0 ... if the user has configured eth0 with NetworkManager or the distro's networking scripts. Yes, I think option 2 is pretty much doomed to fail. If you don't integrate with the native distro mgmt scripts, or with NetworkManager then you are forever going to have a fight between who's managing what. You can already see this with Xen's network-bridge script which cna be considered option 2. If you restart your network using the distro scripts, after Xen's network-bridge script has run, then you end up wwith a completely trashed host network. All in all, option (1) seems more attractive, since it should save us from dealing with a lot of low-level details of network setup, and the distro scripts should be much better integrated with the rest of the system than what we come up with for libvirt. IMHO, (1) and (3) are requirements and we'll probably end up doing (2) too in the long run. I think (2) should really be avoided - integration with the OS is of primary importance to avoid getting things into a total mess. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 01:41:19PM +, Daniel P. Berrange wrote: I don't think that's much of an argument. Plenty of things can be considered fundamental. My kernel version certainly is, so why isn't libvirt letting me upgrade that? What about my firewall? Why isn't libvirt configuring my iSCSI target for me? The kernel version isn't fundamental to the task of provisioning and configuring a guest VM. When deploying a VM there is no general requirement to upgrade the host kernel. When deploying a VM there very much is a requirement to configure physical resources in the host such as storage, and networking. So it sounds like you think libvirt /should/ be going out and configuring shared storage. What about VLAN set up on my router hardware? That too, right? The existance of many different impls is exactly the reason for libvirt to have this capability. Libvirt is providing a consistent mgmt API No - it's exactly the reason for SOME common API. No-one is arguing that a common API for host networking is a bad idea. There isn't an API, and it's sometimes needed for management is not an argument for it be part of libvirt's scope. Again, what makes libvirt a good place for this management? I don't accept because it's there as a reasonable justification... regards, john -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] sharing multiple identical USB devices?
Guido � wrote: Qemu/KVM allow to either use vendor/product or address/bus so with two identical USB devices you need to resort to address/bus only. Does dropping vendor/product from the XML completely help? Thanks, Guido! That was the problem! You can not use vendor/product and bus/device at the same time. - Mike -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 18/25: Dynamic thread workers pool
Daniel P. Berrange berra...@redhat.com wrote:
On Fri, Jan 16, 2009 at 12:10:35PM +, Richard W.M. Jones wrote:
On Tue, Jan 13, 2009 at 05:46:08PM +, Daniel P. Berrange wrote:
@@ -1948,6 +2000,26 @@ static int qemudRunLoop(struct qemud_ser
}
}
+/* If number of active workers exceeds both the min_workers
+ * threshold and the number of clients, then kill some
+ * off */
+for (i = 0 ; (i server-nworkers
+ server-nactiveworkers server-nclients
+ server-nactiveworkers min_workers) ; i++) {
+
+if (server-workers[i].active
+!server-workers[i].processing) {
+server-workers[i].quit = 1;
+
+virCondBroadcast(server-job);
+virMutexUnlock(server-lock);
+pthread_join(server-workers[i].thread, NULL);
+virMutexLock(server-lock);
+server-workers[i].active = 0;
+server-nactiveworkers--;
+}
+}
+
Doesn't this cause the main loop to hang -- eg. if we happen to try to
kill of a worker which is doing some lengthy operation?
If the worker is currently processing an RPC call, its 'processing'
flag will be set, and thus we won't consider it a candidate for
killing off.
+struct qemud_worker {
+pthread_t thread;
+int active :1;
+int processing :1;
+int quit : 1;
I guess maybe I'm unclear about the meaning of these flags. What's
the difference between active processing?
I'll add comments to this struct before commiting. Their meanings
are:
active: the thread actually exists
processing: the thread is currently processing an RPC call
Please consider using some name other than active,
since it is so close in meaning to processing.
Maybe something like created, live or alive?
quit: the thread has been asked to quit
At startup we malloc enough 'struct qemud_worker' objects to hold the
entire 'max_workers' set, but we only start threads for 'min_workers.
This is what the 'active' field tracks. The 'processing' flag is there
so that when we want to kill off a thread, we won't block waiting for
a thread that's currently busy working. And the 'quit' flag is that
condition that a thread checks upon waking up from its condition variable
sleep to see if it was asked to quit.
Regards,
Daniel
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 18/25: Dynamic thread workers pool
On Fri, Jan 16, 2009 at 03:07:21PM +0100, Jim Meyering wrote: Please consider using some name other than active, since it is so close in meaning to processing. Maybe something like created, live or alive? Indicates that it has a thread, so how about 'has_thread'? Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my OCaml programming blog: http://camltastic.blogspot.com/ Fedora now supports 68 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
Just to be clear - I am very sympathetic to the need for this stuff and the conclusion that it belongs in libvirt. I just think we need to be fairly clear on where the line is. On Fri, 2009-01-16 at 13:41 +, Daniel P. Berrange wrote: On Fri, Jan 16, 2009 at 01:24:53PM +, John Levon wrote: On Fri, Jan 16, 2009 at 10:11:02AM +, Daniel P. Berrange wrote: Integrating with host networking meanwhile is a fundamental requirement for virtualization for all apps using libvirt, since guests need network connectivity, and thus managing NICs should be within scope. I don't think that's much of an argument. Plenty of things can be considered fundamental. My kernel version certainly is, so why isn't libvirt letting me upgrade that? What about my firewall? Why isn't libvirt configuring my iSCSI target for me? The kernel version isn't fundamental to the task of provisioning and configuring a guest VM. When deploying a VM there is no general requirement to upgrade the host kernel. There may be a requirement to install certain packages before VMs can be deployed. When deploying a VM there very much is a requirement to configure physical resources in the host such as storage, and networking. When deploying a VM or when configuring a host to run VMs? i.e. is this isn't an API to list the available bridges which a user could choose to connect a VM to, this is an API to configure bonded NICs etc. The Virtual Network and Storage Pool stuff is different as well - it's about virtualizing those resources. Configuring a bonded NIC or setting a static IP address on eth0 is not about virtualizing eth0. With the exception, perhaps, of configuring a NIC to be a shared physical interface, this stuff seems to me to be about host configuration rather than helping with VM configuration or virtualizing resources. ... What remote management tools can benefit from is piggy backing on top of libvirt's authenticated connection to a host. What would be so wrong with adding a TCP stream tunnel (c.f. SSH tunnel) over the libvirt connection and allow the management tool talk to a separate host configuration agent? Cheers, Mark. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 02:20:27PM +, Mark McLoughlin wrote: Just to be clear - I am very sympathetic to the need for this stuff and the conclusion that it belongs in libvirt. I just think we need to be fairly clear on where the line is. On Fri, 2009-01-16 at 13:41 +, Daniel P. Berrange wrote: On Fri, Jan 16, 2009 at 01:24:53PM +, John Levon wrote: On Fri, Jan 16, 2009 at 10:11:02AM +, Daniel P. Berrange wrote: Integrating with host networking meanwhile is a fundamental requirement for virtualization for all apps using libvirt, since guests need network connectivity, and thus managing NICs should be within scope. I don't think that's much of an argument. Plenty of things can be considered fundamental. My kernel version certainly is, so why isn't libvirt letting me upgrade that? What about my firewall? Why isn't libvirt configuring my iSCSI target for me? The kernel version isn't fundamental to the task of provisioning and configuring a guest VM. When deploying a VM there is no general requirement to upgrade the host kernel. There may be a requirement to install certain packages before VMs can be deployed. That is true, you also need to installl an OS before you can manage a physical host, and you need to build a machine before you can install an OS on it. We could go on for ever here, which is why I'm trying to make define the boundary line as being related to the management of hardware resources on the host. WRT the need to install certain packages for, say KVM, libvirt's role is to provide information about what capabilities are present. So the host XML for capablities indicates whether KVM virtualization is available. Libvirt is managing what is available on the host, not trying to install more thing. Installing RPMs is not hardware resource management, hence I class it out of scope. When deploying a VM there very much is a requirement to configure physical resources in the host such as storage, and networking. When deploying a VM or when configuring a host to run VMs? Configuring resourcs on the host to connect to a VM. So finding or creating a LVM LUN, creating a VLAN devices on NIC and putting it into a bridge to guest to a guest. i.e. is this isn't an API to list the available bridges which a user could choose to connect a VM to, this is an API to configure bonded NICs etc. Mere listing of host devices is already provided by the node device enumation APIs. The Virtual Network and Storage Pool stuff is different as well - it's about virtualizing those resources. Configuring a bonded NIC or setting a static IP address on eth0 is not about virtualizing eth0. I don't think there is such a distinction between the storage pool stuff and the network stuff. In storage pools we are taking phys devices and putting them into a LVM volume groups - this is directly akin to taking two NICs and bonding them. Likewise managing FibreChanel with NPIV is directly akin to managing VLANs on a NIC. So I class configuration of bonding, bridges and VLANs within scope fo the network mgmt API. What remote management tools can benefit from is piggy backing on top of libvirt's authenticated connection to a host. What would be so wrong with adding a TCP stream tunnel (c.f. SSH tunnel) over the libvirt connection and allow the management tool talk to a separate host configuration agent? That isn't providing an API that is consistnet across virt manangement platforms. It is not providing an API and XML model that is consistnet with the rest of the model exposed through libvirt's hypervisor, node capability, node device virtual network APIs. Network interface APIs are the core missing piece of libvirt API functionality IMHO. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 01:24:53PM +, John Levon wrote: We should be considering why libvirt is /well-placed/ to configure the host. I think it should be pretty clear that it's actually not: the problems around distro differences alone is a good indication. The proposed API is anaemic enough to not be of much use. This is precisely why it is merely a proposal - if you can elaborate on why is insufficiently useful we can discuss how to address the shortcomings or whether the extra needs should be declared out of scope. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Allow virtual networks to survive daemon restarts
On Wed, Jan 14, 2009 at 12:44:50PM +, Daniel P. Berrange wrote:
On Wed, Jan 14, 2009 at 08:55:46AM +, Mark McLoughlin wrote:
On Tue, 2009-01-13 at 20:49 +, Daniel P. Berrange wrote:
Currently when we shutdown the virtual networks are all shutdown too.
This is less than useful if we're letting guest VMs hang around post
shutdown of libvirtd, because it means we're tearing their network
connection out from under them. This patch fixes that allowing networks
to survive restarts, and be re-detected next time around.
When starting a virtual network we write the live config into
/var/lib/libvirt/network/$NAME.xml
This is because the bridge device name is potentially auto-generated
and we need to keep track of that
We change dnsmasq args to include an explicit pidfile location
/var/run/libvirt/network/$NAME.pid
and also tell it to put itself into the background - ie daemonize. This
is because we want dnsmasq to survive the daemon.
Now, when libvirtd starts up it
- Looks for the live config, and if found loads it.
- Calls a new method brHasBridge() to see if its desired bridge
actually exists (and thus whether the network is running).
If it exists,the network is marked active
- If DHCP is configured, then reads the dnsmasq PIDfile, and sends
kill($PID, 0) to check if the process is actually alive
In addition I cleanup the network code to remove the configFile and
autostartLink fields in virNetworkObjPtr, so it matches virDomaiObjPtr
usage.
With all this applied you can now restart the daemon, and virbr0 is
left happily running.
It all sounds and looks good to me. Some comments below, but nothing
major.
Here's an updated patch with all your comments incorporated.
Sounds a real user improvement, patch looks fine but I'm concerned
about this:
+/* Finally try and read dnsmasq pid if any DHCP ranges are set */
+if (obj-def-nranges
+virFileReadPid(NETWORK_PID_DIR, obj-def-name,
+ obj-dnsmasqPid) == 0) {
+
+/* Check its still alive */
+if (kill(obj-dnsmasqPid, 0) != 0)
+obj-dnsmasqPid = -1;
+
+/* XXX ideally we'd check this was actually
+ * the dnsmasq process, not a stale pid file
+ * with someone else's process. But how ?
+ */
OS specific but check in configure about the location of dnsmasq
export it as DNSMASQ_PATH, then compare to file pointed by /proc/pid/exe
#ifdef __linux__
char *pidpath;
virAsprintf(pidpath, /proc/%d/exe, obj-dnsmasqPid);
if (virFileLinkPointsTo(pidpath, DNSMASQ_PATH) == 0))
obj-dnsmasqPid = -1;
VIR_FREE(pidpath);
#endif
I'm sure one can find a Solaris equivalent.
Patch looks fine to me,
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] RFC: configuring host interfaces with libvirt
On Fri, Jan 16, 2009 at 01:13:18AM +, David Lutterkort wrote:
1. XML Format
=
The first question is how the user would describe the host interfaces they
want. Below are some sketches of what an XML representation of the various
kinds of interfaces would look like. This covers the minimal amount of
settings for these to be useful, though I am sure we'll need to add more
over time.
interface device=eth0 onboot=yes
hwaddr00:19:d2:9f:88:96/hwaddr
dhcp peerdns=yes/
/interface
interface device=eth1 onboot=yes
hwaddr00:19:d2:9f:88:97/hwaddr
static ipaddr=192.168.0.5 gateway=192.168.0.1 netmask=255.255.255.0/
/interface
interface device=eth2 onboot=yes
hwaddr00:19:d2:9f:88:98/hwaddr
/interface
bond name=bond00 onboot=yes mode=active-backup
slave device=eth0 primary=yes/
slave device=eth1/
/bond
bridge name=br0 stp=off onboot=yes
member device=eth2/
dhcp peerdns=yes/
/bridge
vlan device=eth0 tag=42 reorder_hdr=yes/
From a style point of view I'd prefer these to all have the same
top level XML element name, and use type='phys|bond|vlan|bridge'
attribute for distinction, since this follows the pattern we use
in the XML for other objects we describe. Type speciic sub-elements
can be used where there are specific extra metadat pieces we need
for that type. Also prefer to see it follow the syntax
used elsewhere for mac addresses, and finally agreed with te idea
that 'onboot' should actually be handled by the get/set-autostart
API, again to match the model used elsehwere in the APIs.
interface type='physical'
nameeth0/name
mac address='00:19:d2:9f:88:96'/
dhcp peerdns=yes/
/interface
interface type='physical'
nameeth1/name
mac address='00:19:d2:9f:88:97'/
ip address=192.168.0.5 gateway=192.168.0.1 netmask=255.255.255.0/
/interface
interface type='physical'
nameeth2/name
mac address='00:19:d2:9f:88:98'/
/interface
interface type='bond'
namebond0/name
bond mode='active-backup'
interface name=eth0 primary=yes/
interface name=eth1/
/bond
/bond
interface type='bridge'
namebr0/name
bridge stp='off'
interface name=eth2/
/bridge
dhcp peerdns=yes/
/bridge
interface type='vlan'
namevlan0/name
vlan tag=42 reorder_hdr=yes
interface name='eth0'
/vlan
/inteface
All of these should also allow a uuid element for specifying a uuid; I
omitted that for brevity.
2. API Changes
==
There are two options for dealing with network interfaces: (1) use the
existing virNetwork* calls or (2) add completely new API calls.
Repurposing existing virNetwork* calls
--
The existing calls map well to the operations we need for managing
interfaces, with a few exceptions:
- virNetworkGetAutostart/SetAutostart: depending on how we implement all
this (see below), 'autostart' might actually mean 'on boot', not 'when
libvirtd starts'
- virNetworkGetBridgeName doesn't make sense for interfaces, and should
return NULL for interfaces
We'll probably also end up adding some functions to query details about an
interface, in particular, a call to see what kind of network/interface a
virNetworkPtr represents
I think the fact that the XML description of the devices is quite
different, and the underling impl will be quite different, suggests
that re-using existing APIs is not worthwhile.
Add completely new virInterface* calls
--
This would add roughly the same API calls as the virNetwork* calls,
i.e. we'd have something like
typedef struct virInterface *virInterfacePtr;
int virInterfaceCreate(virInterfacePtr);
virInterfacePtr virInterfaceCreateXML(..);
...
plus some calls to extract information from a virInterfacePtr
The second option seems cleaner to me and easier to implement, and avoids
any subtle changes in the behavior of existing API, though I don't like
that we'll be adding another 20 or so calls to libvirt's public API, with
attendant churn both in the drivers and in virsh.
I think the number of public API entry points is not worth
worrying about, since that's such a tiny part of the code
effort. Any saving from reusing the virNetwork API entry
points is dwarfed by the code to actually implemnent these
network capabilities internally.
4. Misc issues
==
* Should interfaces have labels/roles ('data-interface') to help admins
make sense of the current config ?
Those are application defined semantics really, so I don't think
they need direct representation in libvirt. It would also entail
an extra look-aside config cache, because initscript / networkmanager
(or other backend impl) don't provide any means to store such
labels/roles.
* Do we expect interfaces to be in a specific state before we create them
or do we just tear them down and reconfigure them no matter what ?
I think we should expect the
Re: [libvirt] PATCH: 8/25: Concurrent dispatch of RPC methods
Daniel P. Berrange wrote: On Fri, Jan 16, 2009 at 12:49:06PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: This patch re-writes the code for dispatching RPC calls in the remote driver to allow use from multiple threads. Only one thread is allowed to send/recv on the socket at a time though. If another thread comes along it will put itself on a queue and go to sleep. The first thread may actually get around to transmitting the 2nd thread's request while it is waiting for its own reply. It may even get the 2nd threads reply, if its own RPC call is being really slow. So when a thread wakes up from sleeping, it has to check whether its own RPC call has already been processed. Likewise when a thread owning the socket finishes with its own wor, it may have to pass the buck to another thread. The upshot of this, is that we have mutliple RPC calls executing in parallel, and requests+reply are no longer guarenteed to be FIFO on the wire if talking to a new enough server. This refactoring required use of a self-pipe/poll trick for sync between threads, but fortunately gnulib now provides this on Windows too, so there's no compatability problem there. snip +goto error; +} +xdr_destroy (xdr); + +return rv; + +error: +VIR_FREE(ret); +return NULL; The above should free rv, not ret: VIR_FREE(rv); Doh, bad naming convention for arguments - we always use 'ret' for return values. I should rename the argument to 'reply' since its what contains the RPC reply, so we can use the normal convention of 'ret' for return value. I actually just tracked this down independently: the above bug was crashing virt-manager, where we incorrectly were passing 'None' to interfaceStats occasionally. So, good catch Jim :) Thanks, Cole -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 8/25: Concurrent dispatch of RPC methods
On Fri, Jan 16, 2009 at 10:18:32AM -0500, Cole Robinson wrote: Daniel P. Berrange wrote: On Fri, Jan 16, 2009 at 12:49:06PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: This patch re-writes the code for dispatching RPC calls in the remote driver to allow use from multiple threads. Only one thread is allowed to send/recv on the socket at a time though. If another thread comes along it will put itself on a queue and go to sleep. The first thread may actually get around to transmitting the 2nd thread's request while it is waiting for its own reply. It may even get the 2nd threads reply, if its own RPC call is being really slow. So when a thread wakes up from sleeping, it has to check whether its own RPC call has already been processed. Likewise when a thread owning the socket finishes with its own wor, it may have to pass the buck to another thread. The upshot of this, is that we have mutliple RPC calls executing in parallel, and requests+reply are no longer guarenteed to be FIFO on the wire if talking to a new enough server. This refactoring required use of a self-pipe/poll trick for sync between threads, but fortunately gnulib now provides this on Windows too, so there's no compatability problem there. snip +goto error; +} +xdr_destroy (xdr); + +return rv; + +error: +VIR_FREE(ret); +return NULL; The above should free rv, not ret: VIR_FREE(rv); Doh, bad naming convention for arguments - we always use 'ret' for return values. I should rename the argument to 'reply' since its what contains the RPC reply, so we can use the normal convention of 'ret' for return value. I actually just tracked this down independently: the above bug was crashing virt-manager, where we incorrectly were passing 'None' to interfaceStats occasionally. So, good catch Jim :) That mistake should never have got anywhere near the RPC code! We are supposed to test all mandatory parameter for non-NULL in the libvirt.c file. I see that the 'path' arg in virDomainInterfaceStats is not being checked for non-NULL, so that's another bug to fix. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 19/25: Remove use of non-threadsafe POSIX apis
On Fri, Jan 16, 2009 at 12:25:02PM +, Daniel P. Berrange wrote:
On Fri, Jan 16, 2009 at 12:21:59PM +, Richard W.M. Jones wrote:
On Tue, Jan 13, 2009 at 05:46:37PM +, Daniel P. Berrange wrote:
+char buf[1024];
sysconf (_SC_GETPW_R_SIZE_MAX)?
Looking at glibc's implementation of getpwuid (which uses getpwuid_r),
I see that glibc dynamically reallocates the buffer as necessary to
the correct size for the return value. The logic of this is fairly
simple so maybe we should do the same?
From glibc:
buffer = malloc (/*some_initial_size*/);
while (buffer != NULL
(getpwuid_r (const char *name, resbuf, buffer,
buffer_size, result)
== ERANGE))
{
char *new_buf;
buffer_size *= 2;
new_buf = (char *) realloc (buffer, buffer_size);
if (new_buf == NULL)
{
free (buffer);
errno = ENOMEM;
}
buffer = new_buf;
}
Anyhow, +1 but I'd be happier if these functions were centralized in
somewhere like src/utils.c.
That's a good idea - in all the cases where we currently use getpwuid
all we actually want is the home directory path. So we could add a
simple func:
char *virUserHomeDirectory(uid_t uid);
and hide all the horrific code in there.
yes +1, patch looks sane to me but that would be an improvement !
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: 21/25: Use random_r for random numbers
On Tue, Jan 13, 2009 at 05:47:27PM +, Daniel P. Berrange wrote: Now that gnulib's rand module is imported, we have a decent quality random number generator that's portable. We don't want to mess with the apps state, so in virInitialize() we explicitly initialize our own private random nubmer generator state with virRandomInitialize(). The util.h file gains a convenience macro, since random_r() is horrible to call and we need to protect our global state int virRandom(int max) +1 looks fine ! 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: 22/25: Make configure output aligned
On Fri, Jan 16, 2009 at 12:24:36PM +, Richard W.M. Jones wrote: On Tue, Jan 13, 2009 at 05:47:49PM +, Daniel P. Berrange wrote: The configure output looked untidy, so this aligns things correctly. configure.in | 20 ++-- 1 file changed, 10 insertions(+), 10 deletions(-) Cleanup always good. +1 trivial, +1 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: 24/25: Update python error reporting
On Tue, Jan 13, 2009 at 05:48:40PM +, Daniel P. Berrange wrote: The virGetLastError() method is now stored in a thread local and virConnGetLastError() is deprecated, so switch to only use the former. Also, no need to copy the error object since it is thread local already thus guarenteed not to change behind our backs okay, fine, +1, 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: 25/25: Support ac97 sound card
On Tue, Jan 13, 2009 at 05:49:02PM +, Daniel P. Berrange wrote: QEMU now has support for a sound card of type ac97, so enable that in the XML parser / qemu driver. Not related to the threading really, looks fine. Any automatic way to detect unused functions ? I guess we can't expect the compiler or linker to help in this case since we make them public for internal consumption. 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] Fix vm define error with back compat console device
Daniel Veillard veill...@redhat.com wrote: On Fri, Jan 16, 2009 at 12:09:33AM +, Daniel P. Berrange wrote: On Thu, Jan 15, 2009 at 10:17:56PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: ... + virsh --connect qemu:///session define devs.xml Shouldn't use qemu:///session for test cases like this - this is what the test:///default driver is for, avoiding the fragility danger of using the daemon live hypervisor drivers. There's no failure with test:///default. I'm rather surprised at that - both drivers use identical XML formating routines here, so given the same config, both should fail just as badly. Is this perhaps a case where we need to run it under valgrind to make it reliably fail. Maybe that can be debugged separately, I would like to see the patch fixed, maybe we can isolate the problem in the test driver (if any) but it should not block the initial patch from being commited, Cole, can you commit your fix? As soon as that's in, I'll commit the following simple test to exercise it: It's similar to a previous patch, but with the following changes - uses test:///default driver rather than the qemu: one, per Daniel Berrange's suggestion - adjusts its input XML so that it's identical to dumpxml output I'm sure Dan and I will be talking about bigger, long-term testing framework changes in the next week or so. Jim From bc7d653b1c7039da3edb63528a83bdc9609c7b6f Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Thu, 15 Jan 2009 19:51:39 +0100 Subject: [PATCH] exercise a bug that could make libvirtd segfault * tests/define-dev-segfault: New file. * tests/Makefile.am (test_scripts): Add define-dev-segfault. --- tests/Makefile.am |1 + tests/define-dev-segfault | 76 + 2 files changed, 77 insertions(+), 0 deletions(-) create mode 100755 tests/define-dev-segfault diff --git a/tests/Makefile.am b/tests/Makefile.am index c735d62..39b5727 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -60,6 +60,7 @@ if WITH_LIBVIRTD test_scripts +=\ cpuset \ daemon-conf \ + define-dev-segfault \ int-overflow\ libvirtd-fail \ libvirtd-pool \ diff --git a/tests/define-dev-segfault b/tests/define-dev-segfault new file mode 100755 index 000..4ae286f --- /dev/null +++ b/tests/define-dev-segfault @@ -0,0 +1,76 @@ +#!/bin/sh +# Exercise a bug whereby defining a valid domain could kill libvirtd. +# The bug can also be exercised with a simple define/dumpxml pair to virsh. + +if test $VERBOSE = yes; then + set -x + virsh --version +fi + +test -z $srcdir srcdir=$(pwd) +test -z $abs_top_srcdir abs_top_srcdir=$(pwd)/.. +. $srcdir/test-lib.sh + +fail=0 + +# Domain definition from Cole Robinson. +cat \EOF D.xml || fail=1 +domain type='kvm' + nameD/name + uuidaaa3ae22-fed2-bfbd-ac02-3bea3bcfad82/uuid + memory262144/memory + currentMemory262144/currentMemory + vcpu1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='cdrom'/ + /os + features +acpi/ + /features + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu-kvm/emulator +serial type='pty' + target port='0'/ +/serial +serial type='pty' + target port='1'/ +/serial +serial type='pty' + target port='2'/ +/serial +parallel type='pty' + target port='0'/ +/parallel +parallel type='pty' + target port='1'/ +/parallel +parallel type='pty' + target port='2'/ +/parallel +console type='pty' + target port='0'/ +/console +sound model='pcspk'/ +sound model='es1370'/ + /devices +/domain +EOF + +url=test:///default +virsh --connect $url 'define D.xml; dumpxml D' out 21 || fail=1 + +cat exp EOF || fail=1 +Domain D defined from D.xml + +$(cat D.xml) + +EOF + +compare exp out || fail=1 + +exit $fail -- 1.6.1.258.g7ff14 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix vm define error with back compat console device
Jim Meyering wrote: Daniel Veillard veill...@redhat.com wrote: On Fri, Jan 16, 2009 at 12:09:33AM +, Daniel P. Berrange wrote: On Thu, Jan 15, 2009 at 10:17:56PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: ... + virsh --connect qemu:///session define devs.xml Shouldn't use qemu:///session for test cases like this - this is what the test:///default driver is for, avoiding the fragility danger of using the daemon live hypervisor drivers. There's no failure with test:///default. I'm rather surprised at that - both drivers use identical XML formating routines here, so given the same config, both should fail just as badly. Is this perhaps a case where we need to run it under valgrind to make it reliably fail. Maybe that can be debugged separately, I would like to see the patch fixed, maybe we can isolate the problem in the test driver (if any) but it should not block the initial patch from being commited, Cole, can you commit your fix? Pushed now. Thanks, Cole -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 8/25: Concurrent dispatch of RPC methods
Jim Meyering j...@meyering.net wrote: Daniel P. Berrange berra...@redhat.com wrote: This patch re-writes the code for dispatching RPC calls in the remote driver to allow use from multiple threads. Only one thread is allowed to send/recv on the socket at a time though. If another thread comes along it will put itself on a queue and go to sleep. The first thread may actually get around to transmitting the 2nd thread's request while it is waiting for its own reply. It may even get the 2nd threads reply, if its own RPC call is being really slow. So when a thread wakes up from sleeping, it has to check whether its own RPC call has already been processed. Likewise when a thread owning the socket finishes with its own wor, it may have to pass the buck to another thread. The upshot of this, is that we have mutliple RPC calls executing in parallel, and requests+reply are no longer guarenteed to be FIFO on the wire if talking to a new enough server. This refactoring required use of a self-pipe/poll trick for sync between threads, but fortunately gnulib now provides this on Windows too, so there's no compatability problem there. Quick summary: dense ;-) though lots of moved code. I haven't finished,... Modulo the things I mentioned, and the following nits, the rest looked fine. Typo in a comment: $ g grep -n EGAIN src/remote_internal.c:6110: * EGAIN printing a raw errno value (rather than a strerror-style string) in a DEBUG statement: $ g grep -n 'DEB.*, errno' src/remote_internal.c:6190: \ DEBUG(Poll unexpectedly failed %d\n, errno); comment typos: +/* Two reasons we can be woken up + * 1. Other thread has got our reply ready for us + * 2. Other thread is all done, and its out turn to + * be the dispatcher to finish waiting for + * out reply + */ s/its out/it's our/ s/out reply/our reply/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Fix vm define error with back compat console device
Cole Robinson crobi...@redhat.com wrote: Jim Meyering wrote: Daniel Veillard veill...@redhat.com wrote: On Fri, Jan 16, 2009 at 12:09:33AM +, Daniel P. Berrange wrote: On Thu, Jan 15, 2009 at 10:17:56PM +0100, Jim Meyering wrote: Daniel P. Berrange berra...@redhat.com wrote: ... + virsh --connect qemu:///session define devs.xml Shouldn't use qemu:///session for test cases like this - this is what the test:///default driver is for, avoiding the fragility danger of using the daemon live hypervisor drivers. There's no failure with test:///default. I'm rather surprised at that - both drivers use identical XML formating routines here, so given the same config, both should fail just as badly. Is this perhaps a case where we need to run it under valgrind to make it reliably fail. Maybe that can be debugged separately, I would like to see the patch fixed, maybe we can isolate the problem in the test driver (if any) but it should not block the initial patch from being commited, Cole, can you commit your fix? Pushed now. Thanks! I've pushed the following two changes and rebased the git branch: From 1c8099e9e4bfc4c1c3ca2f003822040d72c9cfcf Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Fri, 16 Jan 2009 18:06:33 + Subject: [PATCH 1/2] tests: exercise a bug that could make virsh and libvirtd segfault * tests/define-dev-segfault: New file. * tests/Makefile.am (test_scripts): Add define-dev-segfault. --- ChangeLog |6 +++ tests/Makefile.am |1 + tests/define-dev-segfault | 76 + 3 files changed, 83 insertions(+), 0 deletions(-) create mode 100755 tests/define-dev-segfault diff --git a/ChangeLog b/ChangeLog index 7655836..536c563 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,9 @@ +Fri Jan 16 18:41:40 +0100 2009 Jim Meyering meyer...@redhat.com + + tests: exercise a bug that could make virsh and libvirtd segfault + * tests/define-dev-segfault: New file. + * tests/Makefile.am (test_scripts): Add define-dev-segfault. + Fri Jan 16 11:48:41 EST 2009 Cole Robinson crobi...@redhat.com * src/domain_conf.c: Fix segfault with console device back compat. diff --git a/tests/Makefile.am b/tests/Makefile.am index 0486ee3..98739c5 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -61,6 +61,7 @@ test_scripts += \ test_conf.sh \ cpuset \ daemon-conf \ + define-dev-segfault \ int-overflow \ read-bufsiz \ read-non-seekable \ diff --git a/tests/define-dev-segfault b/tests/define-dev-segfault new file mode 100755 index 000..4ae286f --- /dev/null +++ b/tests/define-dev-segfault @@ -0,0 +1,76 @@ +#!/bin/sh +# Exercise a bug whereby defining a valid domain could kill libvirtd. +# The bug can also be exercised with a simple define/dumpxml pair to virsh. + +if test $VERBOSE = yes; then + set -x + virsh --version +fi + +test -z $srcdir srcdir=$(pwd) +test -z $abs_top_srcdir abs_top_srcdir=$(pwd)/.. +. $srcdir/test-lib.sh + +fail=0 + +# Domain definition from Cole Robinson. +cat \EOF D.xml || fail=1 +domain type='kvm' + nameD/name + uuidaaa3ae22-fed2-bfbd-ac02-3bea3bcfad82/uuid + memory262144/memory + currentMemory262144/currentMemory + vcpu1/vcpu + os +type arch='i686' machine='pc'hvm/type +boot dev='cdrom'/ + /os + features +acpi/ + /features + clock offset='utc'/ + on_poweroffdestroy/on_poweroff + on_rebootrestart/on_reboot + on_crashdestroy/on_crash + devices +emulator/usr/bin/qemu-kvm/emulator +serial type='pty' + target port='0'/ +/serial +serial type='pty' + target port='1'/ +/serial +serial type='pty' + target port='2'/ +/serial +parallel type='pty' + target port='0'/ +/parallel +parallel type='pty' + target port='1'/ +/parallel +parallel type='pty' + target port='2'/ +/parallel +console type='pty' + target port='0'/ +/console +sound model='pcspk'/ +sound model='es1370'/ + /devices +/domain +EOF + +url=test:///default +virsh --connect $url 'define D.xml; dumpxml D' out 21 || fail=1 + +cat exp EOF || fail=1 +Domain D defined from D.xml + +$(cat D.xml) + +EOF + +compare exp out || fail=1 + +exit $fail -- 1.6.1.258.g7ff14 From 0337ba238e69322750d3036f6794798ff1ed80e5 Mon Sep 17 00:00:00 2001 From: Jim Meyering meyer...@redhat.com Date: Fri, 16 Jan 2009 18:07:24 + Subject: [PATCH 2/2] tests: virsh-all and virsh-synopsis were not being run * tests/Makefile.am (test_scripts): Add two missing backslashes. --- ChangeLog |5 - tests/Makefile.am | 24 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/ChangeLog b/ChangeLog index 536c563..689ec9e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,4 +1,7 @@ -Fri Jan 16 18:41:40 +0100 2009 Jim Meyering meyer...@redhat.com +Fri Jan 16 18:44:08 +0100 2009 Jim Meyering
Re: [libvirt] PATCH: 9/25: Make global error object thread local
Daniel P. Berrange berra...@redhat.com wrote:
The virGetLastError() and virConnGetLastError() methods are not
Dan,
I suppose you're using mercurial to produce these diffs.
If so, you may want to file a bug, since the diffs
are misleadingly suboptimal. Here's one example:
-
-DEBUG(conn=%p, conn);
+DEBUG(conn=%p, conn);
+
+virResetLastError();
That's not so bad: since they're short and right next
to each other, it's obvious they're identical.
However, I saw the following one first, and searched visually
for a while before using the editor to confirm that the added/removed
virLibConnError (conn, VIR_ERR_NO_SUPPORT...
lines are identical:
@@ -1217,15 +1235,25 @@ virConnectGetHostname (virConnectPtr con
{
DEBUG(conn=%p, conn);
-if (!VIR_IS_CONNECT(conn)) {
-virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
-return NULL;
-}
-
-if (conn-driver-getHostname)
-return conn-driver-getHostname (conn);
-
-virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+virResetLastError();
+
+if (!VIR_IS_CONNECT(conn)) {
+virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+return NULL;
+}
+
+if (conn-driver-getHostname) {
+char *ret = conn-driver-getHostname (conn);
+if (!ret)
+goto error;
+return ret;
+}
+
+virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+/* Copy to connection error object for back compatability */
+virSetConnError(conn);
return NULL;
}
--
It's hard enough to read some of these changes without such duplication.
For reference, here are the git-generated diffs:
@@ -1217,15 +1235,25 @@ virConnectGetHostname (virConnectPtr conn)
{
DEBUG(conn=%p, conn);
+virResetLastError();
+
if (!VIR_IS_CONNECT(conn)) {
virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
return NULL;
}
-if (conn-driver-getHostname)
-return conn-driver-getHostname (conn);
+if (conn-driver-getHostname) {
+char *ret = conn-driver-getHostname (conn);
+if (!ret)
+goto error;
+return ret;
+}
virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+/* Copy to connection error object for back compatability */
+virSetConnError(conn);
return NULL;
}
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] patch: allow disk cache mode to be specified in a domain's xml definition.
We have found certain application scenarios where
overriding of the default qemu host cache mode
provides a substantial improvement in guest
performance. In particular, disabling host caching
of the file/dev backing a guest drive. A summary
of performance metrics may be found below.
Attached is a simple patch which adds general
support for a cache=* attribute to be passed
to qemu from libvirt as part of a xml driver
element. It is parsed in a domain's xml definition
as well as being generated during output of the
same. For example:
disk type='file' device='disk'
source file='/guest_roots/fc8.root'/
target dev='hda'/
driver name='qemu' type='qwerty' cache='none'/
/disk
where both the existing type=* and added cache=*
attributes are optional and independent.
Note this is only intended to serve as an internal
hook allowing override of qemu's default cache
behavior. At the present it is not proposed as an
end-user documented/visible feature.
-john
Mark Wagner wrote:
This data is provided is provided by the Red Hat Performance
team. It is intended to support the request for adding the
required functionality to libvirt to allow for setting a
storage parameter to control the caching behavior of QEMU.
This value would be passed on the QEMU command line.
Our testing of commercial databases in a larger, Enterprise
configuration (MSA, 16 cores, etc) has shown that the default
QEMU caching behavior is not always the best. This set of data
compares the drop off in performance of both writethrough
and writeback cache compared to the nocache option. The main
metric is transactions per minute (tpm). The other axis is
the number of simulated users (x1000)
% nocache TPM Results
K Users WT WB
10 60% 64%
20 66% 71%
40 68% 72%
60 71% 75%
80 74% 79%
100 76% 83%
From the above set of data, it is clear that the default behavior
of QEMU is the worst performer out of the three cache options for
this type of use case. It is also clear that we at minimum, 25%
of the possible TPM performance just due to the cache setting.
--
john.coo...@redhat.com
domain_conf.c | 32 +---
domain_conf.h | 15 +++
qemu_conf.c | 32
3 files changed, 68 insertions(+), 11 deletions(-)
=
--- a/src/domain_conf.h
+++ b/src/domain_conf.h
@@ -80,6 +80,20 @@ enum virDomainDiskBus {
VIR_DOMAIN_DISK_BUS_LAST
};
+/* summary of all possible host open file cache modes
+ */
+typedef enum {
+VIR_DOMAIN_DISK_CACHE_DEFAULT = 0,
+VIR_DOMAIN_DISK_CACHE_DISABLE,
+VIR_DOMAIN_DISK_CACHE_WRITEBACK,
+VIR_DOMAIN_DISK_CACHE_WRITETHRU,
+} host_cachemode;
+
+typedef struct {
+char *tag;
+host_cachemode idx;
+} cache_map_t;
+
/* Stores the virtual disk configuration */
typedef struct _virDomainDiskDef virDomainDiskDef;
typedef virDomainDiskDef *virDomainDiskDefPtr;
@@ -91,6 +105,7 @@ struct _virDomainDiskDef {
char *dst;
char *driverName;
char *driverType;
+host_cachemode cachemode;
unsigned int readonly : 1;
unsigned int shared : 1;
int slotnum; /* pci slot number for unattach */
=
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -616,6 +616,26 @@ qemudNetworkIfaceConnect(virConnectPtr c
return NULL;
}
+/* map from internal cache mode to qemu cache arg text
+ */
+static cache_map_t qemu_cache_map[] = {
+{none, VIR_DOMAIN_DISK_CACHE_DISABLE},
+{writeback, VIR_DOMAIN_DISK_CACHE_WRITEBACK},
+{writethrough, VIR_DOMAIN_DISK_CACHE_WRITETHRU},
+{NULL}};
+
+/* return qemu arg text * corresponding to idx if found, NULL otherwise
+ */
+static inline char *qemu_cache_mode(host_cachemode idx)
+{
+int i;
+
+for (i = 0; qemu_cache_map[i].tag; ++i)
+ if (qemu_cache_map[i].idx == idx)
+return (qemu_cache_map[i].tag);
+return (NULL);
+}
+
static int qemudBuildCommandLineChrDevStr(virDomainChrDefPtr dev,
char *buf,
int buflen)
@@ -946,6 +966,8 @@ int qemudBuildCommandLine(virConnectPtr
virDomainDiskDefPtr disk = vm-def-disks[i];
int idx = virDiskNameToIndex(disk-dst);
const char *bus = virDomainDiskQEMUBusTypeToString(disk-bus);
+int nc;
+char *cachemode;
if (disk-bus == VIR_DOMAIN_DISK_BUS_USB) {
if (disk-device == VIR_DOMAIN_DISK_DEVICE_DISK) {
@@ -980,15 +1002,17 @@ int qemudBuildCommandLine(virConnectPtr
break;
}
-snprintf(opt, PATH_MAX, file=%s,if=%s,%sindex=%d%s%s,
+nc = snprintf(opt, PATH_MAX, file=%s,if=%s,%sindex=%d%s,
disk-src ? disk-src : , bus,
media ? media
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, 2009-01-16 at 03:29 +, John Levon wrote: On Fri, Jan 16, 2009 at 02:59:17AM +, David Lutterkort wrote: I am not disagreeing with you, but either way, libvirt needs _some_ way to control host interfaces. This is far from obvious to me. Could you explain more? The reasoning goes - similar to what Mark said - something like: virtualization is concerned with splitting up the physical resources of a host and presenting them as virtual resources to guests. Physical resources are (at a minimum) CPU, memory, storage and networking; in order to split up resources, we need very fine-grained and low-level control over them - we have that in libvirt for CPU and memory (via the hypervisor) and for storage through libvirt's storage API. Adding API to control host interfaces rounds that picture out by adding fine-grained control for the last major physical resource libvirt isn't managing yet. David -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, 2009-01-16 at 15:06 +, Daniel P. Berrange wrote: On Fri, Jan 16, 2009 at 01:13:18AM +, David Lutterkort wrote: From a style point of view I'd prefer these to all have the same top level XML element name, and use type='phys|bond|vlan|bridge' attribute for distinction, since this follows the pattern we use in the XML for other objects we describe. Type speciic sub-elements can be used where there are specific extra metadat pieces we need for that type. Agreed - I initially thought you couldn't describe that with Relax-NG (element content being completely different depending on the value of an attribute), but found out that you actually can do that. I think the fact that the XML description of the devices is quite different, and the underling impl will be quite different, suggests that re-using existing APIs is not worthwhile. Agreed - that seems to be the consensus. * Do we expect interfaces to be in a specific state before we create them or do we just tear them down and reconfigure them no matter what ? I think we should expect the mgmt app to have put the device in suitable state. eg, if you're creating a bridge containing eth0, we should expect that the eth0 has been taken offline already. What's the mgmt app in this context ? I am thinking that if you tell libvirt to change the config of eth0, it should just nuke whatever config it may already have. * Are there crucial config options that are not covered by the sketches above (e.g., setting an explicit MTU) ? Are there things in the XML sketches above that will be impossible to implement on some OS ? There are almost certainly things we won't be able to implement for certain backends. XenAPI's API does not allow for VLANs for example. This brings up an important wrinkle: selection of the 'interface' driver will mostly be independent of the HV, since it depends more on the OS/distro you are running as your host than on your HV. I've been hoping we could just sidestep Xen's network handling altogether and just focus on doing the right thing for the host platform, regardless of HV. Since we need that for kvm anyway, dealing with the networking part of Xen would just add redundant driver implementations. David -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
Hi Mark, On Fri, 2009-01-16 at 09:00 +, Mark McLoughlin wrote: Hi David, On Fri, 2009-01-16 at 01:13 +, David Lutterkort wrote: bond name=bond00 onboot=yes mode=active-backup slave device=eth0 primary=yes/ slave device=eth1/ /bond bridge name=br0 stp=off onboot=yes member device=eth2/ dhcp peerdns=yes/ /bridge I don't think we want to define a bridge here, but more that an interface is shared - i.e. this is a property of eth2. The main concern is that this is the way I'd expect NetworkManager to support it - i.e. that you could configure NetworkManager to share eth0, rather than ask it to create br0 and add eth0 to it. If you just want to create a bridge, you can creati a virtual network. Is it possible to use DHCP to configure the bridge device itself using that, similar to what's described in [1] ? And have guest's DHCP requests forwarded across the bridge ? The docs only talk about static IP assignments for the bridge device. vlan device=eth0 tag=42 reorder_hdr=yes/ I think these last three are also interfaces and should be described with an interface tag e.g. Agreed. I don't think I like this much - these APIs manage a virtual network which I see as a distinct concept from configuring host hardware. Really, I think keeping the two things separate will actually reduce confusion in the long run. Agreed. This sounds good, but interface is pretty damn generic :-) virNetInterface virNetDevice ... Sure, virInterface is pretty generic; maybe virNetDevice, though we'd have to call it net-device in the XML for consistency. Naming is NP-hard ;) What I don't like about any of these is that I've always imagined we might add further APIs to libvirt for changing a domain's configuration without munging XML e.g. int virDomainGetInterfaces(virDomainPtr domain, virInterfacePtr interfaces, int *ninterfaces); int virInterfaceSetMacAddress(virInterfacePtr interface, const uint8_t mac[6]); So, you can see the potential namespace conflicts ... Not really, since this still follows the naming convention 'vir' + class_name + method_name. There's a virDomainInterfaceStats, and it doesn't seem all that confusing to me to distinguish that from a virInterfaceStats (i.e. stats for all interfaces on the host) 3. Implementation = Configuring network interfaces is highly OS and OS-variant/distro dependant. There are at least two different ways how libvirt can go about modifying the host to create interfaces: 1. Modify the system's network setup scripts (ifcfg-XXX on RH) 2. Directly use the system's network utilities like ifconfig 3. Rely on NetworkManager (not an option right now, as NM doesn't know about bridges and the like) It has to be an option - even if it only supports a subset of what the other options support. I really wouldn't like to see us push ahead with this until we have a plan for how this will work with NetworkManager (and a rough agreement for how future support for bonding etc. in NetworkManager might be configured) Yeah, I started talking to Dan Williams about their plans etc. Of course, their main use case (wireless networking) is unimportant for libvirt. But we should be able to find a way to share some of the backend bits and the general model for interfaces, umm, net-devices ;) The thing is, this has to integrate with existing configuration - there's no point in futzing about with ip link set eth0 ... if the user has configured eth0 with NetworkManager or the distro's networking scripts. Agreed. If we want 'autostart' for an interface to mean 'bring up the interface as soon as the system boots', we are pretty much forced to go with option (1). Why? Because those are what brings up networking at boot - otherwise we'd define 'autostart' as 'whenever libvirtd starts' - but it doesn't seem like nay of this is controversial, anyway. David [1] http://wiki.libvirt.org/page/Networking#Fedora.2FRHEL_Bridging -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
On Fri, 2009-01-16 at 13:49 +, Daniel P. Berrange wrote: On Fri, Jan 16, 2009 at 09:00:16AM +, Mark McLoughlin wrote: The thing is, this has to integrate with existing configuration - there's no point in futzing about with ip link set eth0 ... if the user has configured eth0 with NetworkManager or the distro's networking scripts. Yes, I think option 2 is pretty much doomed to fail. If you don't integrate with the native distro mgmt scripts, or with NetworkManager then you are forever going to have a fight between who's managing what. You can already see this with Xen's network-bridge script which cna be considered option 2. If you restart your network using the distro scripts, after Xen's network-bridge script has run, then you end up wwith a completely trashed host network. That goes for pretty much any config munging task - don't ever change the canonical place where config data is stored; for network devices that's dictated by the distro. Whatever we do for libvirt can only ever be an (updatable) view of that data. David -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] RFC: configuring host interfaces with libvirt
David Lutterkort wrote: For certain applications, we want libvirt to be able to configure host network interfaces in a variety of ways; currently, we are most interested in teaching libvirt how to set up ordinary ethernet interfaces, bridges, bonding and vlan's. I agree that carving up host physical resources, including networking, is appropriate in libvirt. Others have such support in their virutalization APIs an the DMTF is currently working on models in SVPC. I had requests for this functionality in libvirt several months back but no cycles to seriously investigate. [...] 3. Implementation = Configuring network interfaces is highly OS and OS-variant/distro dependant. There are at least two different ways how libvirt can go about modifying the host to create interfaces: 1. Modify the system's network setup scripts (ifcfg-XXX on RH) 2. Directly use the system's network utilities like ifconfig 3. Rely on NetworkManager (not an option right now, as NM doesn't know about bridges and the like) Option (1) saves us from replicating every bit of network setup functionality that those scripts already have - besides configuring the interface, we also might have to setup routes, run dhclient etc. This needs to be supported to play well with all of the existing scripts. I think Daniel P already pointed this out in another response. Option (2) would require far fewer backend implementations than (1) - we should be able to get away with one implementation for Linux, rather than one for Fedora/RHEL, one for Debian, one for SuSe, three for gentoo etc. Xen's network-bridge script uses this approach and it's disastrous IMO. Try rcnetwork restart on a network-bridge configured host :-). In SLE11, we dropped this script and manage host network resources via YaST. But, users want a nice remote interface that will work across heterogeneous hosts ... Your Option 3 needs to be supported as well and is one reason I have not tackled this subject. I don't know anything about NetworkManager and have not had time to dig in. That's about to change with SLE11 reaching late RC phase :-). If we want 'autostart' for an interface to mean 'bring up the interface as soon as the system boots', we are pretty much forced to go with option (1). Right. As in STARTMODE='onboot' All in all, option (1) seems more attractive, since it should save us from dealing with a lot of low-level details of network setup, and the distro scripts should be much better integrated with the rest of the system than what we come up with for libvirt. Although I'm no expert in SuSE sysconfig scripts, I think this approach works best for SuSE distros. I'll have to alert a sysconfig maintainer of this thread. Thanks for raising this topic :-). Jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
