Re: [libvirt] [PATCH] fix MinGW compilation(200808)
Atsushi SAKAI [EMAIL PROTECTED] wrote: Hi, Jim and Dan How about this? src/domain_conf.c|1 + src/domain_conf.h|6 +++--- src/network_conf.c |1 + src/qemu_driver.c| 32 src/util.c |4 ++-- src/virsh.c |3 +++ tests/testutilsxen.c |2 ++ 7 files changed, 28 insertions(+), 21 deletions(-) Thanks Atsushi SAKAI Jim Meyering [EMAIL PROTECTED] wrote: Atsushi SAKAI [EMAIL PROTECTED] wrote: Hi, Dan Thank you for commenting it. Without WITH_QEMU, following errors are appeared. === In file included from test.c:44: domain_conf.h:447: error: syntax error before '' token domain_conf.h:447: warning: no semicolon at end of struct or union domain_conf.h:448: error: syntax error before '' token domain_conf.h:449: error: syntax error before '' token domain_conf.h:458: error: syntax error before ':' token domain_conf.h:459: error: syntax error before ':' token domain_conf.h:468: error: syntax error before '}' token ... +#ifdef WITH_QEMU int stdin; int stdout; int stderr; +#endif Instead of an ifdef, I suggest renaming those three variables. Then their names won't conflict with the names from stdio.h. Please make that var-renaming change a separate change set. I'd prefer names with the _fd suffix (to make it clear they are file descriptors). The vm prefix is a little redundant. Please make the name-changing delta a separate commit: i.e., do not mix it with the other portability-related changes. Thanks, Jim -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH]: hostdev passthrough support take #4
On Fri, Aug 08, 2008 at 05:59:56PM +0200, Guido Günther wrote: Hi, On Fri, Aug 08, 2008 at 10:34:00AM -0400, Daniel Veillard wrote: [..snip..] I think the only thing missing is extending the descrition in the documentation would you mind adding a description in formatdomain.html(.in) Probably an USB devices section added below the elementsDisks part, explaining the syntaxes and stating that it was added in versions after 0.4.4 If you don't feel comfortable with that I can do it, but you know better :) Attached is a short patch for this. This might be a bit terse but we'll have to change this anyhow once pci passthrough for xen arrives. Cheers, That's fine, just fixed a couple of typos and commited, thanks a lot :-) ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
James Morris wrote: On Sun, 10 Aug 2008, Casey Schaufler wrote: 1.1 Rationale With increased use of virtualization, one security benefit of physically separated systems -- strong isolation -- is reduced, This issue can always be readily resolved by going back to physically separated hardware. The strength of isolation of a virtual machine mechanism is one of the important characteristics that should be considered when it is chosen over a hardware isolation scheme, but the systems available today appear to do a pretty good job on this, so I would like to see some evidence of this claim before accepting it as a primary premise. I suspect you misunderstood an important aspect of this in that we are targeting Linux-based virtualization, where the VMs are running inside Linux processes. In this case, the isolation depends on DAC in the host, and the idea behind sVirt is to apply MAC security to these VMs and their resources. Currently, all such VMs run with the same security label, and all of the resources accessed (e.g. disk images) have the same labels. We are simply proposing a mechanism to allow distinct security labels to be applied to these entities, which in the simplest case, will allow MAC policy to enforce isolation between them. Well, let's look at the situation and see what sort of risk we're talking about. A VM running inside a Linux process is subject to the same kinds of vulnerabilities as any other program too be sure. So the problem you are looking to address is that the label of the process is based on the label of the program file. This problem is hardly unique to VMs, as anyone who wants to run two web servers with different MLS values will tell you. ... You are not going to solve any problems of misconfiguration this way, you are just going to make the environment more complicated and prone to misconfiguration. I don't think this is valid as an absolute statement. You're correct. It is a strong opinion that I hold. My strong opinions and $4.55 will get you a nice coffee at Peets. ... I can see where someone who is not familiar with VMs might think this is plausible, but looking at the interfaces and separation provided by VMs makes it pretty clear that this isn't even a belts and braces level of extra protection. I disagree. VMs are software, and all software has bugs. If you can break out of the VM in Linux-based virtualization, you can probably get to all of the other VMs on the system (they are running in the same DAC and MAC contexts). Assuming again that you're using program based MAC. A traditional BL system where the process retains its label through exec() would not have this shortcoming. Applying distinct labels allows MAC policy to be enforced by the host kernel so that such breaches are contained within the isolated host VM process. Or are you saying that you don't think hypervisors can be broken out of ? Not any more than web servers, name servers, or game programs, many of which don't do particularly well in a program based MAC environment, either. o Increased protection of VM guests from each other in the event of host flaws or misconfiguration. Some protection may also be provided against a compromised host. Flaws and misconfiguration will not be helped by this. I don't see why not. With MAC containment, if, say, a web server on the host was compromised, an attacker may be prevented from interfering with the VMs running on the system. This is already true to some extent with coarse-grained MAC. Sure, there are some flaws and misconfigurations that will be caught, but I would never count on it as a security feature in an environment that matters. ... OK, I get the picture. You really want to run VMs under SELinux. Go wild, but I think you are significantly overstating the value and creating a project where a a little bit of policy work ought to handle all but the most extreme cases. The proof of concept code is indeed simple policy/labeling changes, although we want to ensure that we fully understand the requirements, and implement a flexible and generally useful scheme. Support for this also needs to be built directly into the virt toolchain so that the user is provided with a complete solution, rather than a collection of pieces. How do you envision the Virt toolchain changing to support SELinux? I confess to being pretty curious about what you think needs to change. DAC, MAC, Containers, VMs, and separate hardware are all mechanisms for providing (in ascending order) measures of isolation. It makes sense to tighten a DAC system by adding MAC, or running a MAC system under a VM, but to each the sort of isolation it is good at. So, in the case of a Linux-based VM running as a process in a DAC context, we are indeed tightening a DAC system by adding MAC. Yeah, you're
RE: [libvirt] Qemu Monitor
Nope, the monitor is redirected so that libvirt can send commands to it directly. Hi, How are we to ask the monitor to eject a CD-ROM? Thank you. Regards, Rich -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
On Monday 11 August 2008 19:31, James Morris [EMAIL PROTECTED] wrote: I suspect you misunderstood an important aspect of this in that we are targeting Linux-based virtualization, where the VMs are running inside Linux processes. In this case, the isolation depends on DAC in the host, and the idea behind sVirt is to apply MAC security to these VMs and their resources. Currently, all such VMs run with the same security label, and all of the resources accessed (e.g. disk images) have the same labels. http://en.wikipedia.org/wiki/NetTop So it's basically a free implementation of NetTop? an issue which may be ameliorated with the application of Mandatory Access Control (MAC) security in the host system. Ok. I've been using VMs for 30 years and MAC for 20, and to my mind the only way this statement can possibly be true is if both the MAC system and the VM system are inadequate to their respective tasks. I'm not sure how you come to the conclusion that the MAC system must be inadequate, but this scheme does attempt to improve the robustness of isolation between Linux-based VMs. I think that Casey's idea is that if someone breaks the VM separation then you lose it all. For separation based on UML there are obvious benefits to having different labels for processes and files so that if someone cracks the UML kernel then they end up with just a regular user access on the Linux host. Which of course they could then try to crack with any of the usual local-root exploits. For separation based on Xen if someone cracks the hypervisor then you lose everything. For KVM (which seems to be the future of Linux virtualisation) I don't know enough to comment. We're applying the idea of containing applications in the face of potential flaws and misconfiguration to the case of applications which happen to be VMs. In this sense, it is not different to containing any other form of application (e.g. a flaw in the VM might be exploited by malicious code). VMWare has it's own device drivers. Surely someone who wanted to attack a VMWare based system would go for the drivers which have the ability to override any other protection mechanisms. But I think that constraining the user-space code (as done in NetTop) does provide significant benefits. Again, keep in mind that we're talking about Linux-based virtualization, where the VM is literally just another application running in the host OS. So by Linux-based you mean in contrast to Xen which has the Xen kernel (not Linux) running on the hardware? I don't understand what needs to be backed here. Currently, MAC is not used to separate different Linux-based VMs, and by integrating MAC support, people will be able to further utilize MAC. One thing that should be noted is the labelled network benefits. If you had several groups of virtual servers running at different levels and wanted to prevent information leaks then having SE Linux contexts and labelled networking could make things a little easier. I have had some real challenges in managing firewall rules for Xen servers. My general practice is to try and make sure that there is no real need for firewalls between hosts on the same hardware (not that I want it this way - it's what technical and management issues force me to). So for example if I have an ISP Xen server running virtual machines for a number of organisations I make sure that they are either all within a similar trust boundary (IE affiliated groups) or all mutually untrusting (IE other IP addresses in the same net-block are treated the same as random hosts on the net). o Increased protection of the host from untrusted VM guests (e.g. for VM hosting providers, grid/cloud servers etc.). I can see where someone who is not familiar with VMs might think this is plausible, but looking at the interfaces and separation provided by VMs makes it pretty clear that this isn't even a belts and braces level of extra protection. I disagree. VMs are software, and all software has bugs. If you can break out of the VM in Linux-based virtualization, you can probably get to all of the other VMs on the system (they are running in the same DAC and MAC contexts). Applying distinct labels allows MAC policy to be enforced by the host kernel so that such breaches are contained within the isolated host VM process. Or are you saying that you don't think hypervisors can be broken out of ? The issue is whether the hypervisor you care about can be broken out of in that way. It seems that if someone can break out of Xen then you just lose. For KVM I don't know the situation, do you have a good reference for how it works? http://en.wikipedia.org/wiki/Kernel-based_Virtual_Machine The above web page says that KVM is all based in the kernel, in which case why would it be any more resilient than Xen? o Consolidating access to multiple networks which require
Re: [libvirt] [PATCH] Fix python error reporting for some storage operations
On Mon, Aug 11, 2008 at 03:58:55PM -0400, Cole Robinson wrote: In the python bindings, all vir* classes expect to be passed a virConnect object when instantiated. Before the storage stuff, these classes were only instantiated in virConnect methods, so the generator is hardcoded to pass 'self' as the connection instance to these classes. Problem is there are some methods that return pool or vol instances which aren't called from virConnect: you can lookup a storage volume's associated pool, and can lookup volumes from a pool. In these cases passing 'self' doesn't give the vir* instance a connection, so when it comes time to raise an exception crap hits the fan. Rather than rework the generator to accomodate this edge case, I just fixed the init functions for virStorage* to pull the associated connection out of the passed value if it's not a virConnect instance. Okay, I understand, it's a bit funky to check the class but that should work. Applied and commited, thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 3/7:
Daniel P. Berrange [EMAIL PROTECTED] wrote:
...
[ Nice long explanation. ]
It'd be great to put that in the code.
lxc_conf.c | 195
lxc_conf.h | 12
lxc_container.c | 39 +--
lxc_container.h |8
lxc_controller.c | 349 +++-
lxc_controller.h | 12
lxc_driver.c | 662
++-
util.c | 158 +
util.h | 13 +
9 files changed, 870 insertions(+), 578 deletions(-)
...
diff -r 8093fb566748 src/lxc_conf.c
diff -r 8093fb566748 src/lxc_conf.h
--- a/src/lxc_conf.h Fri Aug 01 14:47:33 2008 +0100
+++ b/src/lxc_conf.h Tue Aug 05 12:13:24 2008 +0100
@@ -46,7 +46,6 @@
struct __lxc_net_def {
int type;
char *parentVeth; /* veth device in parent namespace */
-char *containerVeth;/* veth device in container namespace */
char *txName; /* bridge or network name */
lxc_net_def_t *next;
@@ -87,11 +86,10 @@
struct __lxc_vm {
int pid;
int state;
+int monitor;
I like monitor_fd ;-)
diff -r 8093fb566748 src/lxc_container.c
--- a/src/lxc_container.c Fri Aug 01 14:47:33 2008 +0100
+++ b/src/lxc_container.c Tue Aug 05 12:13:24 2008 +0100
@@ -69,6 +69,8 @@
typedef struct __lxc_child_argv lxc_child_argv_t;
struct __lxc_child_argv {
lxc_vm_def_t *config;
+int nveths;
From the looks of all uses, this can be an unsigned type.
Then readers won't wonder if negative values are significant.
+char **veths;
int monitor;
char *ttyPath;
};
@@ -171,8 +173,7 @@
*
* Returns 0 on success or -1 in case of error
*/
-int lxcContainerSendContinue(virConnectPtr conn,
- int control)
+int lxcContainerSendContinue(int control)
I like control_fd, but hesitated to mention it,
since this is a preexisting name.
{
int rc = -1;
lxc_message_t msg = LXC_CONTINUE_MSG;
@@ -180,7 +181,7 @@
writeCount = safewrite(control, msg, sizeof(msg));
if (writeCount != sizeof(msg)) {
-lxcError(conn, NULL, VIR_ERR_INTERNAL_ERROR,
+lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_(unable to send container continue message: %s),
strerror(errno));
goto error_out;
@@ -230,21 +231,21 @@
*
* Returns 0 on success or nonzero in case of error
*/
-static int lxcContainerEnableInterfaces(const lxc_vm_def_t *def)
+static int lxcContainerEnableInterfaces(int nveths,
+char **veths)
{
-int rc = 0;
-const lxc_net_def_t *net;
+int rc = 0, i;
Many style guides recommend against putting multiple ,-separated
declarations on the same line...
...
@@ -337,12 +338,11 @@
int flags;
int stacksize = getpagesize() * 4;
char *stack, *stacktop;
-lxc_child_argv_t args = { def, control, ttyPath };
+lxc_child_argv_t args = { def, nveths, veths, control, ttyPath };
If possible, it'd be nice to make the struct const.
...
@@ -379,8 +379,9 @@
char *stack;
int childStatus;
-if (features LXC_CONTAINER_FEATURE_NET)
+if (features LXC_CONTAINER_FEATURE_NET) {
flags |= CLONE_NEWNET;
+}
Since you're making a change like this, I suspect it's worth adding a
sentence+example to HACKING saying that we prefer to use braces even
when there's only one statement. Out of habit, I tend *not* to use
braces in that case, but have no trouble adapting. Codifying it might
help avoid a little churn.
Let me know and I'll be happy to make the change.
diff -r 8093fb566748 src/lxc_controller.c
...
-int lxcControllerMain(int appPty, int contPty)
+static int lxcControllerMain(int monitor,
+ int client,
+ int appPty,
+ int contPty)
{
int rc = -1;
int epollFd;
@@ -120,15 +166,29 @@
memset(epollEvent, 0x00, sizeof(epollEvent));
epollEvent.events = EPOLLIN|EPOLLET;/* edge triggered */
epollEvent.data.fd = appPty;
-epollEvent.data.u32 = 0;/* fdArray position */
if (0 epoll_ctl(epollFd, EPOLL_CTL_ADD, appPty, epollEvent)) {
lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_(epoll_ctl(appPty) failed: %s), strerror(errno));
goto cleanup;
}
epollEvent.data.fd = contPty;
-epollEvent.data.u32 = 1;/* fdArray position */
if (0 epoll_ctl(epollFd, EPOLL_CTL_ADD, contPty, epollEvent)) {
+lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+ _(epoll_ctl(contPty) failed: %s), strerror(errno));
+goto cleanup;
+}
+
+epollEvent.events = EPOLLIN;
+epollEvent.data.fd = monitor;
+if (0 epoll_ctl(epollFd, EPOLL_CTL_ADD, monitor, epollEvent)) {
+lxcError(NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+
Re: [libvirt] [PATCH] Implement vol delete for disk pools
On Mon, Aug 11, 2008 at 03:58:41PM -0400, Cole Robinson wrote: The patch below implements virStorageVolDelete for volumes on a disk pool. The only interesting thing here is that parted wants a partition number to delete, so we need to peel off the end of the volume's target path which will be of the form '/dev/sda1' or similar (I assume. If not, it's still better than having nothing implemented). Without much understanding of the storage command, I will say the patch looks fine to me, but I would prefer another review :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Qemu Monitor
On Mon, Aug 11, 2008 at 10:52:59AM -0500, Duzenbury, Rich wrote: Nope, the monitor is redirected so that libvirt can send commands to it directly. Hi, How are we to ask the monitor to eject a CD-ROM? Use the 'virsh attach-disk' command - it you give it the name of an existing CDROM device, it will perform a media change operation. 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] introducing source name (for logical storage pools)
On Fri, Aug 08, 2008 at 03:17:52PM -0400, David Lively wrote:
Hi Folks -
This small patch is a proposed prerequisite for the storage pool
discovery patch I submitted last week.
Daniel B proposed having storage pool discovery return a bunch of XML
storage source elements, rather than full pool elements (which
contain target-dependent details like the local pool name and device
or mount path -- things which don't need to be decided unless/until the
pool is defined).
I like his suggestion a lot, and I think it addresses the final
API-definition problem keeping storage pool discovery out of libvirt.
But ... it doesn't quite work for logical storage pools because those
are created from the pool name element (which is the same as the
volume group name).
I will let Daniel B comment on the pure storage aspects of the patch.
The patch looks fine to me except for one thing:
[...]
+if (options-flags VIR_STORAGE_BACKEND_POOL_SOURCE_NAME) {
+ret-source.name = virXPathString(conn, string(/pool/source/name),
ctxt);
+if (ret-source.name == NULL) {
+/* source name defaults to pool name */
+ret-source.name = ret-name;
+}
+}
I'm vary of allocation issues there.
Basically the patch adds a new string field to the record. But I could not see
any deallocation addition in the patch, and the field seems to only be
set by copying another value. Maybe this is fine now, but if we ever update
the field in a different way (which I would expect at some point in the code
evolution) then we will have a leak.
So instead of copying the string pointer directly, I suggest to do a string
dup and in the freeing part of the record , do the VIR_FREE call for it.
Otherwise this looks fine to me
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
[EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Re: [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
On Tue, Aug 12, 2008 at 03:57:46PM +1000, Russell Coker wrote: On Monday 11 August 2008 19:31, James Morris [EMAIL PROTECTED] wrote: I think that Casey's idea is that if someone breaks the VM separation then you lose it all. For separation based on UML there are obvious benefits to having different labels for processes and files so that if someone cracks the UML kernel then they end up with just a regular user access on the Linux host. Which of course they could then try to crack with any of the usual local-root exploits. For separation based on Xen if someone cracks the hypervisor then you lose everything. Xen is out of scope for this initial work, because we don't want to get involved in the hypervisor code, since then we have to deal with the Xen XSM framework too. For KVM (which seems to be the future of Linux virtualisation) I don't know enough to comment. In KVM, virtual machines really are just processes as far as the host OS is concerned. It is very similar to UML in this respect, except you don't need to have a special kernel image for your VM. Again, keep in mind that we're talking about Linux-based virtualization, where the VM is literally just another application running in the host OS. So by Linux-based you mean in contrast to Xen which has the Xen kernel (not Linux) running on the hardware? By Linux-based we mean a virtualization platform where Linux *is* the hypervisor. This includes KVM, UML. It specifically excludes Xen, since it has a separate hypervisor underneath the host kernel. That's not to say the work couldn't be extended to Xen later, its merely not a core focus. The issue is whether the hypervisor you care about can be broken out of in that way. It seems that if someone can break out of Xen then you just lose. For KVM I don't know the situation, do you have a good reference for how it works? http://en.wikipedia.org/wiki/Kernel-based_Virtual_Machine The above web page says that KVM is all based in the kernel, in which case why would it be any more resilient than Xen? The best way is to thing of KVM, as an accelerator for QEMU. If you're not already familiar, QEMU provides CPU emulation, and device emulation for a wide variety of platforms. The KVM kernel module basically provides a simple API to userspace which allows QEMU's CPU emulation to be switched out in favour of using hardware virtualization capabilities available from latest generation CPUs. The QEMU device emulation is still used. People typically claim that Xen is more resilient than KVM because it has a separate hypervisor and thus has a smaller trusted codebase. In practice this is smoke mirrors, because to do anything useful Xen still has this Dom0 host kernel which has access to all hardware. So I wouldn't claim either Xen or KVM are inherantly more secure than each other. o Consolidating access to multiple networks which require strong isolation from each other (e.g. military, government, corporate extranets, internal Chinese wall separation etc.) The VMs provide this. Isolation is easy. Sharing is what's hard. Again, it's important to understand that these VMs are merely Linux processes and are only currently afforded the same level of isolation as standard DAC. How does the VMs are merely Linux processes fit with the description of KVM? Or are you talking about some other virtualisation system? Again think of the KVM kernel module as simply a CPU accelerator for QEMU. A VM is just a QEMU process which is using KVM for its CPU virtualization. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Re: [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
On Tue, 12 Aug 2008, Russell Coker wrote: having different labels for processes and files so that if someone cracks the UML kernel then they end up with just a regular user access on the Linux host. Which of course they could then try to crack with any of the usual local-root exploits. For separation based on Xen if someone cracks the hypervisor then you lose everything. For KVM (which seems to be the future of Linux virtualisation) I don't know enough to comment. KVM uses a modified version of Qemu where guests run as Linux processes. There are some useful documents here: http://kvm.qumranet.com/kvmwiki/Documents (The OLS paper especially). So by Linux-based you mean in contrast to Xen which has the Xen kernel (not Linux) running on the hardware? Yes. I don't understand what needs to be backed here. Currently, MAC is not used to separate different Linux-based VMs, and by integrating MAC support, people will be able to further utilize MAC. One thing that should be noted is the labelled network benefits. If you had several groups of virtual servers running at different levels and wanted to prevent information leaks then having SE Linux contexts and labelled networking could make things a little easier. I have had some real challenges in managing firewall rules for Xen servers. My general practice is to try and make sure that there is no real need for firewalls between hosts on the same hardware (not that I want it this way - it's what technical and management issues force me to). So for example if I have an ISP Xen server running virtual machines for a number of organisations I make sure that they are either all within a similar trust boundary (IE affiliated groups) or all mutually untrusting (IE other IP addresses in the same net-block are treated the same as random hosts on the net). Thanks for the insights -- we expect to address the virtual networking aspect in some way. The issue is whether the hypervisor you care about can be broken out of in that way. It seems that if someone can break out of Xen then you just lose. For KVM I don't know the situation, do you have a good reference for how it works? http://en.wikipedia.org/wiki/Kernel-based_Virtual_Machine The above web page says that KVM is all based in the kernel, in which case why would it be any more resilient than Xen? KVM uses a kernel module to utilize the virt hardware (which Qemu interfaces with via /dev/kvm), but the guest runs in a userspace process. I'm not comparing which is more resilient. - James -- James Morris [EMAIL PROTECTED] -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Implement vol delete for disk pools
On Mon, Aug 11, 2008 at 03:58:41PM -0400, Cole Robinson wrote:
The patch below implements virStorageVolDelete for volumes
on a disk pool.
The only interesting thing here is that parted wants a
partition number to delete, so we need to peel off the
end of the volume's target path which will be of the form
'/dev/sda1' or similar (I assume. If not, it's still
better than having nothing implemented).
Thanks,
Cole
diff --git a/src/storage_backend_disk.c b/src/storage_backend_disk.c
index dac827b..28e0a52 100644
--- a/src/storage_backend_disk.c
+++ b/src/storage_backend_disk.c
@@ -22,11 +22,13 @@
*/
#include config.h
+#include string.h
#include internal.h
#include storage_backend_disk.h
#include util.h
#include memory.h
+#include c-ctype.h
enum {
VIR_STORAGE_POOL_DISK_DOS = 0,
@@ -416,13 +418,6 @@ virStorageBackendDiskBuildPool(virConnectPtr conn,
return 0;
}
-
-static int
-virStorageBackendDiskDeleteVol(virConnectPtr conn,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol,
- unsigned int flags);
-
static int
virStorageBackendDiskCreateVol(virConnectPtr conn,
virStoragePoolObjPtr pool,
@@ -486,14 +481,41 @@ virStorageBackendDiskCreateVol(virConnectPtr conn,
static int
virStorageBackendDiskDeleteVol(virConnectPtr conn,
- virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
- virStorageVolDefPtr vol ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol,
unsigned int flags ATTRIBUTE_UNUSED)
{
-/* delete a partition */
-virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
- _(Disk pools are not yet supported));
-return -1;
+char *part_num = NULL;
+int i;
+
+/* Strip target path (ex. '/dev/sda1') of its partition number */
+for (i = (strlen(vol-target.path)-1); i = 0; --i) {
+if (!c_isdigit(vol-target.path[i]))
+break;
+part_num = (vol-target.path[i]);
+}
+
+if (!part_num) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot parse partition number from target
+path '%s'), vol-target.path);
+return -1;
+}
This isn't correct because the target path is not guarenteed to point to
the master device name /dev/sda1. The user could have configured it to
use a stable path such as /dev/disk/by-uuid/4cb23887-0d02-4e4c-bc95-7599c85afc1a
So, you need to first call 'readlink' on the vol-target.path, ignoring
EINVAL which you'll get if it wasn't a symlink. Once you've done that
you'll need to validate that it is sane by checking that vol-source.devices[0]
is a prefix of the resolved pathname. Finally, you can extract the partition
number. So something along the lines of
char devname[PATH_MAX];
if (readlink(vol-target.path, devname, sizeof(devname)) 0
errno != EINVAL)
virStorageReportError(...)
if (!STRPREFIX(devname, vol-source.devices[0].path))
virStorageReportError()
part_num = devname + strlen(vol-source.devices[0].path)
+
+/* eg parted /dev/sda rm 2 */
+const char *prog[] = {
+PARTED,
+pool-def-source.devices[0].path,
+rm,
+--script,
+part_num,
+NULL,
+};
+
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 python error reporting for some storage operations
On Mon, Aug 11, 2008 at 03:58:55PM -0400, Cole Robinson wrote: In the python bindings, all vir* classes expect to be passed a virConnect object when instantiated. Before the storage stuff, these classes were only instantiated in virConnect methods, so the generator is hardcoded to pass 'self' as the connection instance to these classes. Problem is there are some methods that return pool or vol instances which aren't called from virConnect: you can lookup a storage volume's associated pool, and can lookup volumes from a pool. In these cases passing 'self' doesn't give the vir* instance a connection, so when it comes time to raise an exception crap hits the fan. Rather than rework the generator to accomodate this edge case, I just fixed the init functions for virStorage* to pull the associated connection out of the passed value if it's not a virConnect instance. I see what you mean. This is actually the fix I would have done anyway - changing the generator in any more complex way is just a huge time sink :-) So ACK Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Change disk type 'dos' to 'msdos'
On Mon, Aug 11, 2008 at 10:01:16AM -0400, Cole Robinson wrote: Jim Meyering wrote: Cole Robinson [EMAIL PROTECTED] wrote: parted doesn't seem to want the label 'dos', instead wanting 'msdos'. Patch is basically s/dos/msdos/ Hi Cole, This definitely needs to be fixed. FYI, dos appears to be the preferred name for that partition table type (google for partition table and either dos or msdos; also, partx --type accepts dos, not msdos). Too bad Parted added the ms prefix. If it's not hard to implement, it would be nice to hide the implementation detail that Parted happens to call it the msdos label type. We could just keep the outward facing label the same, and internally map it to use msdos if calling 'parted mklabel' since it's the only command that seems to use that value. Whatever people think is best. Yep, I'd prefer that we just special case it internally when calling out to mklabel. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Announcement of a companion project the UnifiedSessionsManager
Hello, I would like to announce the unifiedSessionsManager(GPL3, including the claimed inventions), which is based for Xen on the virsh-tool, but basically uses a different approach. I guess it could be a quite usable companion of libvirt for advanced usage and enhanced customizabelity of the handling of huge amounts of virtual and physical machines in a distributed environment. It is targeting on test, development and productive environments. This requirement will particularly arise, when bulk-core-CPUs enter the market, providing for independently executed bulks of virtual machines, which may be logically designed as stacked and nested VMs, founding a tree like multi-layered structure of VM clusters. The design-preview as estimated by the unifiedSessionsManager is here the v-component, what could be seen as the extensin of the slogan/concept of a so called virtual appliance, where a full scale VM including it's inherent GuestOS is handeled as a virtual software component. V-Compoents are foreseen to be grouped into clusters of VMs, arranged within a nested VMSTACK and managed - e.g. suspended - as a group of services, founding a combined service by their self. In case this preview becomes reality, this may have an impact on the basic design, particularly the interface design of each of it's components to be stack-aware, thus for libvirt too. For example providing the shutdown of a stack of VMs, including the suspension of machines containing upper VMs, might require some kind of communications and propagation.. The usage of multilayered VMSTACKs is already supported by the unifiedSessionsManager. Particularly MACRO and GROUP features support for handling of the number of entities required to be handled for a VMSTACK. Whereas much of the required functionality from the VMs is simulated, particularly by utilizing a recursive Stack-Propagation based on native access to GuestOSs with SSO-accounts. The concrete supported products of current version 01_07_001b02 are: - GuestOS and HostOS on physical machines: CentOS, Fedora, debian, OpenSUSE, Ubuntu, OpenBSD, Solaris10(limited) - Hypervisors/Emulators: QEMU, Xn, VMware(workstation,server,player) Future Plans are, mostly for the next version already: -Planned-OS: FreeBSD, NetBSD, Solaris10(complete), OpenSolaris, ScientificLinux -Planned-OS: eCos(?), uCLinux(several ARCH), SkyEye, ... -Planned-VM: VirtualBOX(evaluation first), OpenVZ(evaluation first) .Introduction of a LDAP based nameservice for VMs. -Evaluation of:Cygwin The Idea was the easy to adapt utilization of a minimalistic-bash-scripts only implementation for setting up a unified wrapper interface customizable for almost any kind of user sessions by almost anyone. Even though I would prefer now at least Perl or for specific parts full scale C/C++ code and Java/JavaScript for some administrative parts. The managed user environment comprises sessions to VMs which could be allocated in a nested multilayered VMSTACK as well as within a PM, which are physical machines - layer-0 entities. Also HOSTs sessions as native access to GuestOSs are integrated. Additionally a simple implemented but with sophisticated functionally equipped cache database is introduced in order to support for hundreds and thousands of virtual machines to be managed, with an average query-response-time for single-level-filters of less than 1second. Results are measured with an average of 0.3-0.8seconds in the ref.-env., even though bash and awk is mainly used only with an Office-Compatible record format. Information could be found within the User-Manual, which has more than 600pages, and within the sources of more than 110.000LinesOfCode. The following sites provide downloads: http://www.heise.de/software/download/unified_sessions_manager_ctys/51630 http://www.unifiedsessionsmanager.org http://www.unifiedsessionsmanager.eu http://sourceforge.net/projects/ctys (sourceforge not yet complete) (berlios follows) There are basically three reasons for sending this Mail: 1. Announcing a new supplemental project focusing on usage of distributed and stacked VMs 2. Getting some feedback and ideas for the future direction of the development of libvirt and the unifiedSessionsManager. 3. Probably initiation of setting up a common overall concept for the future vision of an IT environment and it's integrated handling of VMs. And of course, I would appreciate, if the current version already is of some benefit for the OpenSource community. Yours Sincerely Arno-Can Uestuensoez -- Arno-Can Uestuensoez www.i4p.de / www.i4p.com -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
On Mon, Aug 11, 2008 at 12:17:48PM +1000, James Morris wrote: 4. Design Considerations 4.1 Consensus in preliminary discussion appears to be that adding MAC to libvirt will be the most effective approach. Support may then be extended to virsh, virt-manager, oVirt etc. I can see a couple of immediate items to address in the libvirt space - Need to decide how to ensure the VM is run with the correct security label instead of the default virt_t. Cannot assume that all VMs have disks configured. Some VMs may be PXE boot, and use an NFS/iSCSI root filesystem - this is not visible to the host. Implication is that we can't rely on labelling of disks files to infer the VM's security context. This suggests the domain XML format needs to allow for a security context to be specified at time the VM is defined/created in libvirt. libvirt would have to takes steps to make sure the VM is started with this defined context. An approach of including context in the XML would also allow easy extension to Xen XSM framework in future where you specify a context at time of VM creation, which is passed to the hypervisor. - The storage XML format can already report what label a storage volume currently has. In addition we need to be able to set the label. A few complications... - We may need to set it in several places - ie a VM may be assigned a disk based on a stable path such as /dev/disk/by-uuid/4cb23887-0d02-4e4c-bc95-7599c85afc1a Which is a symlink to the real (unstable) device name /dev/sda1 Clearly need to set label on the real device, but may also ned to change the symlink too ? - We can't add the new label to the SELinux policy directly, because the label needs to be on the unstable device name /dev/sdaXXX which can change across host OS reboots. Do we instead add the info the udev rules, so when /dev is populated at boot time by udev the device nodes get the desired initial labelling ? Or do we manually chcon() the device at the time we boot the VM ? - Some storage types don't allow per-file labelling - eg NFS In those scenarios the storage pool is assigned a label and all volumes inherit it. So, if two VMs are using NFS files and need different labelling, they need to use different directories on the NFS server, so that we can have separate mount points with appropriate labelling for each. 4.2 Initially, sVirt should just work as a means to isolate VMs, with minimal administrative interaction. e.g. an option is added to virt-manager which allows a VM to be designated as isolated, and from then on, it is automatically run in a separate security context, with policy etc. being generated and managed by libvirt. 4.3 We need to consider very carefully exactly how VMs will be launched and controlled e.g. overall security ultimately must be enforced by the host kernel, even though VM launch will be initially controlled by host userspace. 4.4 We need to consider overall management of labeling both locally and in distributed environments (e.g. oVirt), as well as situations where VMs may be migrated between systems, backed up etc. We need to define who/what is responsible for ensuring that all hosts in the cluster have the same policy loaded. Typically libvirt only aims to provide the mechanism, and not constrain what you do with it. So perhaps libvirt needs to merely be able to report on what policy version is loaded as part of host capabilities information. oVirt (or FreeIPA?) would be responsible for using this info, and also ensuring that all hosts have same policy if desired/required. One possible approach may be to represent the security label as the UUID of the guest and then translate that to locally meaningful values as needed. This implies there needs to be some lookup table of UUID - security label mappings on every host in the cluster. This needs to be updated whenever a new VM is created, which is a fairly significant data sync task someone/thing needs to take care of. Would be doable for oVirt or FreeIPA, since they have a network-wide view. virt-manager though has individual host-centric view of things - it doesn't consider the broader picture. 4.5 MAC controls/policy will need to be considered for any control planes (e.g. /dev/kvm). I should probably point out that there are in fact two ways in which KVM/QEMU can be used on a host - The 'system' instance. There is one of these per host, and it currently runs as a privileged user (ie root) - The 'session' instance. There is one of these per user, per host and it runs as the unprivileged user. The
Re: [libvirt] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
On Tue, 12 Aug 2008, Daniel P. Berrange wrote: Do we instead add the info the udev rules, so when /dev is populated at boot time by udev the device nodes get the desired initial labelling ? Or do we manually chcon() the device at the time we boot the VM ? Dan Walsh has mentioned wanting to label the device at VM launch so that MCS labels can be dynamically assigned. This raises some other possible issues such as revoking any existing access (Linux doesn't have general revocation) and having the security of the system depend on whatever is performing the relabel (although we can enforce relabelfrom/relabelto permissions). I wonder if existing work/concepts related to MLS device allocation would be useful here. See: http://sourceforge.net/projects/devallocator/ - James -- James Morris [EMAIL PROTECTED] -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Change disk type 'dos' to 'msdos'
On Tue, Aug 12, 2008 at 11:12:07AM +0100, Daniel P. Berrange wrote: On Mon, Aug 11, 2008 at 10:01:16AM -0400, Cole Robinson wrote: Jim Meyering wrote: Cole Robinson [EMAIL PROTECTED] wrote: parted doesn't seem to want the label 'dos', instead wanting 'msdos'. Patch is basically s/dos/msdos/ Hi Cole, This definitely needs to be fixed. FYI, dos appears to be the preferred name for that partition table type (google for partition table and either dos or msdos; also, partx --type accepts dos, not msdos). Too bad Parted added the ms prefix. If it's not hard to implement, it would be nice to hide the implementation detail that Parted happens to call it the msdos label type. We could just keep the outward facing label the same, and internally map it to use msdos if calling 'parted mklabel' since it's the only command that seems to use that value. Whatever people think is best. Yep, I'd prefer that we just special case it internally when calling out to mklabel. Fine by me, my only worry is that we are somehow breaking the storage XML format as a result, but I don't think this is widely used at this point (at least with MSDos) and best done earlier than later, Applied and commited, thanks ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Ensure parted doesn't prompt if labeling disk
On Mon, Aug 11, 2008 at 09:53:48AM -0400, Cole Robinson wrote: Daniel P. Berrange wrote: On Sun, Aug 10, 2008 at 10:40:30PM -0400, Cole Robinson wrote: Trying to 'build' a disk pool uses 'parted mklabel' which can prompt for confirmation. Patch adds the '--script' option to remove the chance of this happening. Can you double-check this works on RHEL-5 - IIRC, the --script arg is a fairly recent addition to parted. My 5.2 box has it. The --script option is also used elsewhere in the disk code, so I assume it's safe. makes sense and safe, applied and commited, thanks again :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] virDomainInterfaceStats why is there a size?
Another simple question, what is the reasoning about the size field in this call. I would really be a happy boy if anyone said: if you put in path == NULL, it will fill your stats structure up to size interfaces. ...but since this is not the case (yet) what is the reason behind it? Stefan -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] PATCH: Block reset signals when fork/exec'ing children
The LXC patches identified a race condition between fork/exec'ing child
processes and signal handlers.
The process using libvirt can have setup arbitrary signal handlers. In
the libvirtd case we have one attached to SIGCHILD, and the handler writes
to a pipe which is then processeed in the main loop. When you fork() signal
handlers are preserved in the child process, but you may well not want the
signal handlers to be run in the children - for example in LXC we closed
the FD associated the pipe after fork(), and that FD is now asociated
with a socket we use to talk to the container. So if the original signal
handler is run bad stuff will happen in the child.
Now signal handlers will eventually get reset when you exec(), but this
leaves open a race condition window between the fork exec() when we
cannot assume it is safe to run the signal handlers from the parent
So, this changes the virExec() function so that before fork()ing it
blocks all signals. After fork() the parent process can restore its
original signal mask. The child process meanwhile calls sigaction()
to reset all signal handlers to SIG_DFL, and then unblocks all signals.
NB, the child does not restore the parents sig-mask - libvirt can be
called from any app, and we don't want to inherit whatever mask the
app may have setup.
On Linux there is a convenient NSIG constant defined in signal.h
telling us how many signals there are. If this isn't defined I
simply set it to 32 which is enough for UNIX pre-dating the
addition of real-time signals to POSIX. If we find convenient
or more appropriate values for other non-Linux OS we can update
this as needed.
NB I call pthread_sigmask() instead of sigprocmask() when doing
the fork/exec, because we cannot assume that the application
using libvirt is single-threaded thus we only want to block
signals in the thread doing the fork/exec.
Daniel
diff -r 1dbfb08d365d src/util.c
--- a/src/util.cThu Aug 07 16:55:11 2008 +0100
+++ b/src/util.cThu Aug 07 16:55:53 2008 +0100
@@ -37,6 +37,7 @@
#include sys/wait.h
#endif
#include string.h
+#include signal.h
#include c-ctype.h
#ifdef HAVE_PATHS_H
@@ -49,6 +50,10 @@
#include util.h
#include memory.h
#include util-lib.c
+
+#ifndef NSIG
+# define NSIG 32
+#endif
#ifndef MIN
# define MIN(a, b) ((a) (b) ? (a) : (b))
@@ -104,9 +109,23 @@
_virExec(virConnectPtr conn,
const char *const*argv,
int *retpid, int infd, int *outfd, int *errfd, int non_block) {
-int pid, null;
+int pid, null, i;
int pipeout[2] = {-1,-1};
int pipeerr[2] = {-1,-1};
+sigset_t oldmask, newmask;
+struct sigaction sig_action;
+
+/*
+ * Need to block signals now, so that child process can safely
+ * kill off caller's signal handlers without a race.
+ */
+sigfillset(newmask);
+if (pthread_sigmask(SIG_SETMASK, newmask, oldmask) 0) {
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(cannot block signals: %s),
+strerror(errno));
+return -1;
+}
if ((null = open(_PATH_DEVNULL, O_RDONLY)) 0) {
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -122,6 +141,34 @@
goto cleanup;
}
+if (outfd) {
+if(non_block
+ virSetNonBlock(pipeout[0]) == -1) {
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(Failed to set non-blocking file descriptor flag));
+goto cleanup;
+}
+
+if(virSetCloseExec(pipeout[0]) == -1) {
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(Failed to set close-on-exec file descriptor flag));
+goto cleanup;
+}
+}
+if (errfd) {
+if(non_block
+ virSetNonBlock(pipeerr[0]) == -1) {
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(Failed to set non-blocking file descriptor flag));
+goto cleanup;
+}
+if(virSetCloseExec(pipeerr[0]) == -1) {
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(Failed to set close-on-exec file descriptor flag));
+goto cleanup;
+}
+}
+
if ((pid = fork()) 0) {
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_(cannot fork child process: %s), strerror(errno));
@@ -132,33 +179,48 @@
close(null);
if (outfd) {
close(pipeout[1]);
-if(non_block)
-if(virSetNonBlock(pipeout[0]) == -1)
-ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-_(Failed to set non-blocking file descriptor flag));
-
-if(virSetCloseExec(pipeout[0]) == -1)
-ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-_(Failed to set close-on-exec file descriptor flag));
*outfd =
Re: [libvirt] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
On Tue, Aug 12, 2008 at 09:20:41AM -0400, Daniel J Walsh wrote: James Morris wrote: On Tue, 12 Aug 2008, Daniel P. Berrange wrote: Do we instead add the info the udev rules, so when /dev is populated at boot time by udev the device nodes get the desired initial labelling ? Or do we manually chcon() the device at the time we boot the VM ? Dan Walsh has mentioned wanting to label the device at VM launch so that MCS labels can be dynamically assigned. This raises some other possible issues such as revoking any existing access (Linux doesn't have general revocation) and having the security of the system depend on whatever is performing the relabel (although we can enforce relabelfrom/relabelto permissions). I wonder if existing work/concepts related to MLS device allocation would be useful here. See: http://sourceforge.net/projects/devallocator/ - James The experimenting I have done has been around labeling of the virt_image and the process with mcs labels to prevent one process from touching another process/image with a different MCS label. system_u:system_r:qemu_t:s0:c1 can read/write system_u:system_r:virt_image_t:s0:c1 But can not read/write system_u:system_r:virt_image_t:s0:c2 or communicate with process system_u:system_r:qemu_t:s0:c2 The idea would be to have libvirt look at the labeling of the image file and lauch the qemu process with the correct type and matching MCS label. That's not going to fly for VMs without disks in the host - either totally diskless VMs, or VMs using iSCSI/NFS network blockdevices / root FS. 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] Qemu Monitor
-Original Message- From: Daniel P. Berrange [mailto:[EMAIL PROTECTED] Sent: Tuesday, August 12, 2008 4:14 AM To: Duzenbury, Rich Cc: libvir-list@redhat.com Subject: Re: [libvirt] Qemu Monitor On Mon, Aug 11, 2008 at 10:52:59AM -0500, Duzenbury, Rich wrote: Nope, the monitor is redirected so that libvirt can send commands to it directly. Hi, How are we to ask the monitor to eject a CD-ROM? Use the 'virsh attach-disk' command - it you give it the name of an existing CDROM device, it will perform a media change operation. Thank you! I couldn't find that bit of info. Regards, Rich -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
James Morris wrote: On Tue, 12 Aug 2008, Daniel P. Berrange wrote: Do we instead add the info the udev rules, so when /dev is populated at boot time by udev the device nodes get the desired initial labelling ? Or do we manually chcon() the device at the time we boot the VM ? Dan Walsh has mentioned wanting to label the device at VM launch so that MCS labels can be dynamically assigned. This raises some other possible issues such as revoking any existing access (Linux doesn't have general revocation) and having the security of the system depend on whatever is performing the relabel (although we can enforce relabelfrom/relabelto permissions). I wonder if existing work/concepts related to MLS device allocation would be useful here. See: http://sourceforge.net/projects/devallocator/ - James The experimenting I have done has been around labeling of the virt_image and the process with mcs labels to prevent one process from touching another process/image with a different MCS label. system_u:system_r:qemu_t:s0:c1 can read/write system_u:system_r:virt_image_t:s0:c1 But can not read/write system_u:system_r:virt_image_t:s0:c2 or communicate with process system_u:system_r:qemu_t:s0:c2 The idea would be to have libvirt look at the labeling of the image file and lauch the qemu process with the correct type and matching MCS label. So a more advanced image might be labeled system_u:system_r:virt_image_nonet_t:s0:c1 and libvirt would launch system_u:system_r:qemu_nonet_t:s0:c2 I have not looked into which devices on the host machine might need higher levels of protection. In Fedora 9/Rawhide libvirt runs as virtd_t and has a transition rule on qemu_exec_t to qemu_t. So all virtual machines run with system_u:system_r:qemu_t:s0 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
Daniel P. Berrange wrote: On Tue, Aug 12, 2008 at 09:20:41AM -0400, Daniel J Walsh wrote: James Morris wrote: On Tue, 12 Aug 2008, Daniel P. Berrange wrote: Do we instead add the info the udev rules, so when /dev is populated at boot time by udev the device nodes get the desired initial labelling ? Or do we manually chcon() the device at the time we boot the VM ? Dan Walsh has mentioned wanting to label the device at VM launch so that MCS labels can be dynamically assigned. This raises some other possible issues such as revoking any existing access (Linux doesn't have general revocation) and having the security of the system depend on whatever is performing the relabel (although we can enforce relabelfrom/relabelto permissions). I wonder if existing work/concepts related to MLS device allocation would be useful here. See: http://sourceforge.net/projects/devallocator/ - James The experimenting I have done has been around labeling of the virt_image and the process with mcs labels to prevent one process from touching another process/image with a different MCS label. system_u:system_r:qemu_t:s0:c1 can read/write system_u:system_r:virt_image_t:s0:c1 But can not read/write system_u:system_r:virt_image_t:s0:c2 or communicate with process system_u:system_r:qemu_t:s0:c2 The idea would be to have libvirt look at the labeling of the image file and lauch the qemu process with the correct type and matching MCS label. That's not going to fly for VMs without disks in the host - either totally diskless VMs, or VMs using iSCSI/NFS network blockdevices / root FS. Daniel We could store the label to run qemu for a particular image in the libvirt database. But this mechanism would have to match up with the labeling on disk or remote storage. system_u:system_r:qemu_nfs_t:s0:c1 can read/write system_u:system_r:nfs_t:s0 Or you have rules that state if virtd_t wants to start an image labeled nfs_t it will use qemu_nfs_t You could still use the MCS label to prevent processes from attacking each other, but if the remote storage does not support labelling you will not be able to prevent them from attacking each others image files. I think libvirt being SELinux aware and have it decide which context to run qemu at is the important point. The arguement about whether it needs to store the SELinux label in its database or base it off the label of the image can be hashed out later. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
On Tue, Aug 12, 2008 at 09:54:23AM -0400, Daniel J Walsh wrote: Daniel P. Berrange wrote: On Tue, Aug 12, 2008 at 09:20:41AM -0400, Daniel J Walsh wrote: The experimenting I have done has been around labeling of the virt_image and the process with mcs labels to prevent one process from touching another process/image with a different MCS label. system_u:system_r:qemu_t:s0:c1 can read/write system_u:system_r:virt_image_t:s0:c1 But can not read/write system_u:system_r:virt_image_t:s0:c2 or communicate with process system_u:system_r:qemu_t:s0:c2 The idea would be to have libvirt look at the labeling of the image file and lauch the qemu process with the correct type and matching MCS label. That's not going to fly for VMs without disks in the host - either totally diskless VMs, or VMs using iSCSI/NFS network blockdevices / root FS. Daniel We could store the label to run qemu for a particular image in the libvirt database. But this mechanism would have to match up with the labeling on disk or remote storage. Ok, one minor point worth mentioning is that libvirt does not have a general purpose database of configurations. The way VM configuration is stored is hypervisor-specific. In the Xen/OpenVZ/VMWware case we pass the config straight through to XenD which takes care of persisting it. In QEMU/LXC case we store the XML config files in /etc/libvirt. Or you have rules that state if virtd_t wants to start an image labeled nfs_t it will use qemu_nfs_t You could still use the MCS label to prevent processes from attacking each other, but if the remote storage does not support labelling you will not be able to prevent them from attacking each others image files. We don't need to restrict ourselves to a single NFS type qemu_nfs_t/nfs_t. A single NFS server can export many directories each of which can be mounted with a different context. I think libvirt being SELinux aware and have it decide which context to run qemu at is the important point. The arguement about whether it needs to store the SELinux label in its database or base it off the label of the image can be hashed out later. So the important thing is that the label is represented in the libvirt XML format, and this XML config is persisted in a manner which is most applicable for the virtualization driver in question. While I know James wants to target KVM primarily, we need make sure the approach we take doesn't paint ourselves into a corner wrt supporting other virt platforms like Xen. Our guiding rule with libvirt is that for every capability we add, we need to come up with a conceptual model that is applicable to all virtualization drivers, even if we only implement it for one particular driver. 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] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
Daniel P. Berrange wrote: On Tue, Aug 12, 2008 at 09:54:23AM -0400, Daniel J Walsh wrote: Daniel P. Berrange wrote: On Tue, Aug 12, 2008 at 09:20:41AM -0400, Daniel J Walsh wrote: The experimenting I have done has been around labeling of the virt_image and the process with mcs labels to prevent one process from touching another process/image with a different MCS label. system_u:system_r:qemu_t:s0:c1 can read/write system_u:system_r:virt_image_t:s0:c1 But can not read/write system_u:system_r:virt_image_t:s0:c2 or communicate with process system_u:system_r:qemu_t:s0:c2 The idea would be to have libvirt look at the labeling of the image file and lauch the qemu process with the correct type and matching MCS label. That's not going to fly for VMs without disks in the host - either totally diskless VMs, or VMs using iSCSI/NFS network blockdevices / root FS. Daniel We could store the label to run qemu for a particular image in the libvirt database. But this mechanism would have to match up with the labeling on disk or remote storage. Ok, one minor point worth mentioning is that libvirt does not have a general purpose database of configurations. The way VM configuration is stored is hypervisor-specific. In the Xen/OpenVZ/VMWware case we pass the config straight through to XenD which takes care of persisting it. In QEMU/LXC case we store the XML config files in /etc/libvirt. Or you have rules that state if virtd_t wants to start an image labeled nfs_t it will use qemu_nfs_t You could still use the MCS label to prevent processes from attacking each other, but if the remote storage does not support labelling you will not be able to prevent them from attacking each others image files. We don't need to restrict ourselves to a single NFS type qemu_nfs_t/nfs_t. A single NFS server can export many directories each of which can be mounted with a different context. Yes and no. The mountpoint can be labeled differently at the directory level I believe. So you would have to store each image in it's own directory and mount at that level in order for mount -o context to work. I think libvirt being SELinux aware and have it decide which context to run qemu at is the important point. The argument about whether it needs to store the SELinux label in its database or base it off the label of the image can be hashed out later. So the important thing is that the label is represented in the libvirt XML format, and this XML config is persisted in a manner which is most applicable for the virtualization driver in question. While I know James wants to target KVM primarily, we need make sure the approach we take doesn't paint ourselves into a corner wrt supporting other virt platforms like Xen. I probably should have used datastore rather then database. Wherever this data is stored it should be protected as libvirt will be making security decisions based on the data. (Of course this is controlled by kernel policy. the kernel would prevent virtd_t for execing an application as sshd_t for example. Our guiding rule with libvirt is that for every capability we add, we need to come up with a conceptual model that is applicable to all virtualization drivers, even if we only implement it for one particular driver. Isn't libvirt going to be execing q program like qemu_t to run xen images? If yes then this should all work with the defined mechanism. Daniel -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [ANNOUNCE][RFC] sVirt: Integrating SELinux and Linux-based virtualization
On Tue, Aug 12, 2008 at 10:16:35AM -0400, Daniel J Walsh wrote: Daniel P. Berrange wrote: On Tue, Aug 12, 2008 at 09:54:23AM -0400, Daniel J Walsh wrote: Daniel P. Berrange wrote: On Tue, Aug 12, 2008 at 09:20:41AM -0400, Daniel J Walsh wrote: The experimenting I have done has been around labeling of the virt_image and the process with mcs labels to prevent one process from touching another process/image with a different MCS label. system_u:system_r:qemu_t:s0:c1 can read/write system_u:system_r:virt_image_t:s0:c1 But can not read/write system_u:system_r:virt_image_t:s0:c2 or communicate with process system_u:system_r:qemu_t:s0:c2 The idea would be to have libvirt look at the labeling of the image file and lauch the qemu process with the correct type and matching MCS label. That's not going to fly for VMs without disks in the host - either totally diskless VMs, or VMs using iSCSI/NFS network blockdevices / root FS. Daniel We could store the label to run qemu for a particular image in the libvirt database. But this mechanism would have to match up with the labeling on disk or remote storage. Ok, one minor point worth mentioning is that libvirt does not have a general purpose database of configurations. The way VM configuration is stored is hypervisor-specific. In the Xen/OpenVZ/VMWware case we pass the config straight through to XenD which takes care of persisting it. In QEMU/LXC case we store the XML config files in /etc/libvirt. Or you have rules that state if virtd_t wants to start an image labeled nfs_t it will use qemu_nfs_t You could still use the MCS label to prevent processes from attacking each other, but if the remote storage does not support labelling you will not be able to prevent them from attacking each others image files. We don't need to restrict ourselves to a single NFS type qemu_nfs_t/nfs_t. A single NFS server can export many directories each of which can be mounted with a different context. Yes and no. The mountpoint can be labeled differently at the directory level I believe. So you would have to store each image in it's own directory and mount at that level in order for mount -o context to work. Yes, that's what I actually meant - different directories on the NFS server for each set of disk images which need to be separated by security label. Our guiding rule with libvirt is that for every capability we add, we need to come up with a conceptual model that is applicable to all virtualization drivers, even if we only implement it for one particular driver. Isn't libvirt going to be execing q program like qemu_t to run xen images? If yes then this should all work with the defined mechanism. Yes no. In the Xen case, we pass the configuration ontop XenD. This talks to the hypervisor to create the virtual machine. Some Xen guests happen to have a QEMU process to provide an emulated device model, but this isn't required by Xen. We can however pass a security label to XenD and have it do the neccessary security work at VM creation time. Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: 4/7: Convert LXC to new domain APIs
Daniel P. Berrange [EMAIL PROTECTED] wrote: The re-architecting of the LXC controller/container process relationship in the previous patch removed the last obstacle to switching over to the generic domain XML routines. So this patch switches the driver over. First the vast majority of lxc_conf.h/c is simply deleted - this is all redundant when using the domain_conf.h APIs. Then, all references to lxc_vm_t are changed to virDomainObj, and lxc_vm_def_t switches to virDomainDef. Finally the LXC driver registers its capabilities data. For this I have chosen an OS type of 'exe', since the 'operating system' we're running in the container is just any plain executable process. lxc_conf.c | 1052 +-- lxc_conf.h | 121 -- lxc_container.c | 23 - lxc_container.h |2 lxc_controller.c |4 lxc_controller.h |2 lxc_driver.c | 289 --- 7 files changed, 215 insertions(+), 1278 deletions(-) All looks fine. ACK. However, please note that it would have been a lot easier/quicker to review if you'd done the mechanical/automatable changes (i.e., the global substitutions like s/lxc_vm_t/virDomainObj/) separately from the others. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] static ip address
On Fri, Aug 08, 2008 at 09:32:06PM +0200, Olivier Deckmyn wrote: On Sun, Aug 3, 2008 at 7:04 PM, Mads Chr. Olesen [EMAIL PROTECTED] wrote: lør, 02 08 2008 kl. 23:13 +0200, skrev Olivier Deckmyn: I would like to be able to choose on a way or another the ip of each of my VM. As far as I can understand, the only way to get an IP (from a vm point of view) if the dhcp it. But I have no garantee vm1 will always get x.x.x.1 Ip adress... This has been discussed previously, see https://www.redhat.com/archives/libvir-list/2008-April/msg00327.html Could someone please commit this patch, allowing libvirt to handle dnsmasq? Small reality check for you: - did you try to apply it ? doesn't work: that code has completely changed - did you see the I haven't had the time to clean it up properly in the original message ? - libvirt *do* use dnsmasq That said I'm working on a new version of the patch which would be adequate I hope for inclusion, but complaints like this are counter productive and suggestion to blindly apply patches without making sure they work and that the code is in sane state are just improper, sorry ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] static ip address
On Tue, Aug 12, 2008 at 5:17 PM, Daniel Veillard [EMAIL PROTECTED]wrote: On Fri, Aug 08, 2008 at 09:32:06PM +0200, Olivier Deckmyn wrote: On Sun, Aug 3, 2008 at 7:04 PM, Mads Chr. Olesen [EMAIL PROTECTED] wrote: lør, 02 08 2008 kl. 23:13 +0200, skrev Olivier Deckmyn: I would like to be able to choose on a way or another the ip of each of my VM. As far as I can understand, the only way to get an IP (from a vm point of view) if the dhcp it. But I have no garantee vm1 will always get x.x.x.1 Ip adress... This has been discussed previously, see https://www.redhat.com/archives/libvir-list/2008-April/msg00327.html Could someone please commit this patch, allowing libvirt to handle dnsmasq? Many thanx for replying to my mail. Small reality check for you: - did you try to apply it ? doesn't work: that code has completely changed Yes I did try. But I'm not fluent at all with C (I speak Python), and could'nt apply most part of patches, even by hand. - did you see the I haven't had the time to clean it up properly in the original message ? Yes I did. clean seems, from my lazy chair, a simple operation. Sorry if I made a mistake on this. I know pretty well open-source communities, and I know it's not easy (read : boring) to have an external guy claiming for a feature. I wish I could help more :( - libvirt *do* use dnsmasq Yes, and it's the nice news :) That said I'm working on a new version of the patch which would be adequate Thanx a lot. I hope for inclusion, but complaints like this are counter productive and suggestion to blindly apply patches without making sure they work and that the code is in sane state are just improper, sorry ! If I'm the one supposed to complain, I'm really you felt that :( I never thought my reply would be counter-productive. I'm really willing to help :( Once again, if I can do anything to help, I will. Thanx. Olivier. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] rewrite virFileLinkPointsTo
I've rewritten virFileLinkPointsTo to be a lot simpler,
and more importantly, it has far fewer failure points.
If anyone wants to preserve the original behavior that
makes it fail when the first parameter does not specify
a symlink, I can add that. The only difference in behavior
would be when the two files are hard-linked.
In any case, I'll remove the FIXME comment.
The diff below is not very readable. Bottom line,
it replaces a pair of functions (the latter is a stub for mingw)
with this:
#define SAME_INODE(Stat_buf_1, Stat_buf_2) \
((Stat_buf_1).st_ino == (Stat_buf_2).st_ino \
(Stat_buf_1).st_dev == (Stat_buf_2).st_dev)
int virFileLinkPointsTo(const char *checkLink,
const char *checkDest)
{
struct stat src_sb;
struct stat dest_sb;
if (stat (checkLink, src_sb) || stat (checkDest, dest_sb))
return 0;
/* FIXME: if required, lstat checkLink to ensure it's a symlink */
if (! SAME_INODE (src_sb, dest_sb)) {
virLog(Link '%s' does not point to '%s', ignoring\n,
checkLink, checkDest);
return 0;
}
return 1;
}
From 4e70c02e547b078b8f35afd0996b6887e1593afd Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Tue, 12 Aug 2008 19:07:46 +0200
Subject: [PATCH] rewrite virFileLinkPointsTo
* src/util.c (SAME_INODE): Define.
(virFileLinkPointsTo): Rewrite to be more portable and more efficient.
---
src/util.c | 97 ++-
1 files changed, 10 insertions(+), 87 deletions(-)
diff --git a/src/util.c b/src/util.c
index 9f056e0..2905a0a 100644
--- a/src/util.c
+++ b/src/util.c
@@ -397,107 +397,30 @@ int virFileHasSuffix(const char *str,
return STREQ(str + len - suffixlen, suffix);
}
-#ifndef __MINGW32__
+#define SAME_INODE(Stat_buf_1, Stat_buf_2) \
+ ((Stat_buf_1).st_ino == (Stat_buf_2).st_ino \
+(Stat_buf_1).st_dev == (Stat_buf_2).st_dev)
int virFileLinkPointsTo(const char *checkLink,
const char *checkDest)
{
-char dest[PATH_MAX];
-char real[PATH_MAX];
-char checkReal[PATH_MAX];
-int n;
-
-/* read the link destination */
-if ((n = readlink(checkLink, dest, PATH_MAX)) 0) {
-switch (errno) {
-case ENOENT:
-case ENOTDIR:
-return 0;
-
-case EINVAL:
-virLog(File '%s' is not a symlink\n,
- checkLink);
-return 0;
-
-}
-virLog(Failed to read symlink '%s': %s\n,
- checkLink, strerror(errno));
-return 0;
-} else if (n = PATH_MAX) {
-virLog(Symlink '%s' contents too long to fit in buffer\n,
- checkLink);
-return 0;
-}
+struct stat src_sb;
+struct stat dest_sb;
-dest[n] = '\0';
+if (stat (checkLink, src_sb) || stat (checkDest, dest_sb))
+ return 0;
-/* make absolute */
-if (dest[0] != '/') {
-char dir[PATH_MAX];
-char tmp[PATH_MAX];
-char *p;
+/* FIXME: if required, lstat checkLink to ensure it's a symlink */
-strncpy(dir, checkLink, PATH_MAX);
-dir[PATH_MAX-1] = '\0';
-
-if (!(p = strrchr(dir, '/'))) {
-virLog(Symlink path '%s' is not absolute\n, checkLink);
-return 0;
-}
-
-if (p == dir) /* handle unlikely root dir case */
-p++;
-
-*p = '\0';
-
-if (virFileBuildPath(dir, dest, NULL, tmp, PATH_MAX) 0) {
-virLog(Path '%s/%s' is too long\n, dir, dest);
-return 0;
-}
-
-strncpy(dest, tmp, PATH_MAX);
-dest[PATH_MAX-1] = '\0';
-}
-
-/* canonicalize both paths */
-if (!realpath(dest, real)) {
-virLog(Failed to expand path '%s' :%s\n, dest, strerror(errno));
-strncpy(real, dest, PATH_MAX);
-real[PATH_MAX-1] = '\0';
-}
-
-if (!realpath(checkDest, checkReal)) {
-virLog(Failed to expand path '%s' :%s\n, checkDest, strerror(errno));
-strncpy(checkReal, checkDest, PATH_MAX);
-checkReal[PATH_MAX-1] = '\0';
-}
-
-/* compare */
-if (STRNEQ(checkReal, real)) {
+if (! SAME_INODE (src_sb, dest_sb)) {
virLog(Link '%s' does not point to '%s', ignoring\n,
- checkLink, checkReal);
+ checkLink, checkDest);
return 0;
}
return 1;
}
-#else /* !__MINGW32__ */
-
-/* Gnulib has an implementation of readlink which could be used
- * to implement this, but it requires LGPLv3.
- */
-
-int
-virFileLinkPointsTo (const char *checkLink ATTRIBUTE_UNUSED,
- const char *checkDest ATTRIBUTE_UNUSED)
-{
-virLog (_(%s: not implemented\n), __FUNCTION__);
-return 0;
-}
-
-#endif /*! __MINGW32__ */
-
int virFileExists(const char *path)
{
struct stat st;
--
1.6.0.rc2.38.g413e06
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] rewrite virFileLinkPointsTo
On Tue, Aug 12, 2008 at 07:18:00PM +0200, Jim Meyering wrote: I've rewritten virFileLinkPointsTo to be a lot simpler, and more importantly, it has far fewer failure points. If anyone wants to preserve the original behavior that makes it fail when the first parameter does not specify a symlink, I can add that. The only difference in behavior would be when the two files are hard-linked. Nah, that's not a difference we'd encounter with the way we use this API inside libvirt. In any case, I'll remove the FIXME comment. Remove the virLog() function too - the caller should be responsible for reporting errors / logging if it so desires. Aside from that, ACK - this is a very nice simplification. 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] rewrite virFileLinkPointsTo
Daniel P. Berrange [EMAIL PROTECTED] wrote:
On Tue, Aug 12, 2008 at 07:18:00PM +0200, Jim Meyering wrote:
I've rewritten virFileLinkPointsTo to be a lot simpler,
and more importantly, it has far fewer failure points.
If anyone wants to preserve the original behavior that
makes it fail when the first parameter does not specify
a symlink, I can add that. The only difference in behavior
would be when the two files are hard-linked.
Nah, that's not a difference we'd encounter with the way
we use this API inside libvirt.
In any case, I'll remove the FIXME comment.
Remove the virLog() function too - the caller should be
responsible for reporting errors / logging if it so
desires. Aside from that, ACK - this is a very nice
simplification.
Glad to.
I would have done that initially, but was trying
to retain more of the initial semantics.
Thanks for the quick review.
Here's the new version:
/* Return nonzero if checkLink and checkDest
refer to the same file. Otherwise, return 0. */
int virFileLinkPointsTo(const char *checkLink,
const char *checkDest)
{
struct stat src_sb;
struct stat dest_sb;
return (stat (checkLink, src_sb) == 0
stat (checkDest, dest_sb) == 0
SAME_INODE (src_sb, dest_sb));
}
And the patch:
From 3e7df57c236c7d04f4800b104a266949d4334fba Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Tue, 12 Aug 2008 19:07:46 +0200
Subject: [PATCH] rewrite virFileLinkPointsTo
* src/util.c (SAME_INODE): Define.
(virFileLinkPointsTo): Rewrite to be more portable and more efficient.
---
src/util.c | 104 ++--
1 files changed, 10 insertions(+), 94 deletions(-)
diff --git a/src/util.c b/src/util.c
index 9f056e0..fe701cf 100644
--- a/src/util.c
+++ b/src/util.c
@@ -397,107 +397,23 @@ int virFileHasSuffix(const char *str,
return STREQ(str + len - suffixlen, suffix);
}
-#ifndef __MINGW32__
+#define SAME_INODE(Stat_buf_1, Stat_buf_2) \
+ ((Stat_buf_1).st_ino == (Stat_buf_2).st_ino \
+(Stat_buf_1).st_dev == (Stat_buf_2).st_dev)
+/* Return nonzero if checkLink and checkDest
+ refer to the same file. Otherwise, return 0. */
int virFileLinkPointsTo(const char *checkLink,
const char *checkDest)
{
-char dest[PATH_MAX];
-char real[PATH_MAX];
-char checkReal[PATH_MAX];
-int n;
-
-/* read the link destination */
-if ((n = readlink(checkLink, dest, PATH_MAX)) 0) {
-switch (errno) {
-case ENOENT:
-case ENOTDIR:
-return 0;
-
-case EINVAL:
-virLog(File '%s' is not a symlink\n,
- checkLink);
-return 0;
-
-}
-virLog(Failed to read symlink '%s': %s\n,
- checkLink, strerror(errno));
-return 0;
-} else if (n = PATH_MAX) {
-virLog(Symlink '%s' contents too long to fit in buffer\n,
- checkLink);
-return 0;
-}
-
-dest[n] = '\0';
-
-/* make absolute */
-if (dest[0] != '/') {
-char dir[PATH_MAX];
-char tmp[PATH_MAX];
-char *p;
-
-strncpy(dir, checkLink, PATH_MAX);
-dir[PATH_MAX-1] = '\0';
-
-if (!(p = strrchr(dir, '/'))) {
-virLog(Symlink path '%s' is not absolute\n, checkLink);
-return 0;
-}
-
-if (p == dir) /* handle unlikely root dir case */
-p++;
-
-*p = '\0';
-
-if (virFileBuildPath(dir, dest, NULL, tmp, PATH_MAX) 0) {
-virLog(Path '%s/%s' is too long\n, dir, dest);
-return 0;
-}
-
-strncpy(dest, tmp, PATH_MAX);
-dest[PATH_MAX-1] = '\0';
-}
-
-/* canonicalize both paths */
-if (!realpath(dest, real)) {
-virLog(Failed to expand path '%s' :%s\n, dest, strerror(errno));
-strncpy(real, dest, PATH_MAX);
-real[PATH_MAX-1] = '\0';
-}
-
-if (!realpath(checkDest, checkReal)) {
-virLog(Failed to expand path '%s' :%s\n, checkDest, strerror(errno));
-strncpy(checkReal, checkDest, PATH_MAX);
-checkReal[PATH_MAX-1] = '\0';
-}
-
-/* compare */
-if (STRNEQ(checkReal, real)) {
-virLog(Link '%s' does not point to '%s', ignoring\n,
- checkLink, checkReal);
-return 0;
-}
+struct stat src_sb;
+struct stat dest_sb;
-return 1;
+return (stat (checkLink, src_sb) == 0
+ stat (checkDest, dest_sb) == 0
+ SAME_INODE (src_sb, dest_sb));
}
-#else /* !__MINGW32__ */
-
-/* Gnulib has an implementation of readlink which could be used
- * to implement this, but it requires LGPLv3.
- */
-
-int
-virFileLinkPointsTo (const char *checkLink ATTRIBUTE_UNUSED,
- const char *checkDest ATTRIBUTE_UNUSED)
-{
-virLog (_(%s: not implemented\n), __FUNCTION__);
-return 0;
-}
-
-#endif /*! __MINGW32__ */
-
int virFileExists(const char *path)
Re: [libvirt] [PATCH] rewrite virFileLinkPointsTo
On Tue, Aug 12, 2008 at 07:29:20PM +0200, Jim Meyering wrote:
Daniel P. Berrange [EMAIL PROTECTED] wrote:
On Tue, Aug 12, 2008 at 07:18:00PM +0200, Jim Meyering wrote:
I've rewritten virFileLinkPointsTo to be a lot simpler,
and more importantly, it has far fewer failure points.
If anyone wants to preserve the original behavior that
makes it fail when the first parameter does not specify
a symlink, I can add that. The only difference in behavior
would be when the two files are hard-linked.
Nah, that's not a difference we'd encounter with the way
we use this API inside libvirt.
In any case, I'll remove the FIXME comment.
Remove the virLog() function too - the caller should be
responsible for reporting errors / logging if it so
desires. Aside from that, ACK - this is a very nice
simplification.
Glad to.
I would have done that initially, but was trying
to retain more of the initial semantics.
That log message has been on my 'hit list' for a while :-)
Thanks for the quick review.
Here's the new version:
/* Return nonzero if checkLink and checkDest
refer to the same file. Otherwise, return 0. */
int virFileLinkPointsTo(const char *checkLink,
const char *checkDest)
{
struct stat src_sb;
struct stat dest_sb;
return (stat (checkLink, src_sb) == 0
stat (checkDest, dest_sb) == 0
SAME_INODE (src_sb, dest_sb));
}
ACK.
Daniel
--
|: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
|: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
|: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] PATCH: Block reset signals when fork/exec'ing children
Daniel P. Berrange [EMAIL PROTECTED] wrote:
The LXC patches identified a race condition between fork/exec'ing child
processes and signal handlers.
Looks fine modulo a few details:
diff -r 1dbfb08d365d src/util.c
...
@@ -104,9 +109,23 @@
_virExec(virConnectPtr conn,
const char *const*argv,
int *retpid, int infd, int *outfd, int *errfd, int non_block) {
-int pid, null;
+int pid, null, i;
Use pid_t as the type for pid.
Hmm... it'd be good to do the same to preexisting *retpid, (above) too.
int pipeout[2] = {-1,-1};
int pipeerr[2] = {-1,-1};
+sigset_t oldmask, newmask;
+struct sigaction sig_action;
+
+/*
+ * Need to block signals now, so that child process can safely
+ * kill off caller's signal handlers without a race.
+ */
+sigfillset(newmask);
+if (pthread_sigmask(SIG_SETMASK, newmask, oldmask) 0) {
This should test != 0, since the function is specified to return
nonzero upon failure. Two more uses below.
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(cannot block signals: %s),
+strerror(errno));
+return -1;
+}
if ((null = open(_PATH_DEVNULL, O_RDONLY)) 0) {
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
@@ -122,6 +141,34 @@
goto cleanup;
}
+if (outfd) {
+if(non_block
I know this is moved code, but you might want to take the opportunity
to add a space between the if and the following open parenthesis.
+ virSetNonBlock(pipeout[0]) == -1) {
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(Failed to set non-blocking file descriptor
flag));
+goto cleanup;
+}
+
+if(virSetCloseExec(pipeout[0]) == -1) {
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(Failed to set close-on-exec file descriptor
flag));
+goto cleanup;
+}
+}
+if (errfd) {
+if(non_block
+ virSetNonBlock(pipeerr[0]) == -1) {
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(Failed to set non-blocking file descriptor
flag));
+goto cleanup;
+}
+if(virSetCloseExec(pipeerr[0]) == -1) {
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(Failed to set close-on-exec file descriptor
flag));
+goto cleanup;
+}
+}
+
if ((pid = fork()) 0) {
ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
_(cannot fork child process: %s), strerror(errno));
@@ -132,33 +179,48 @@
close(null);
if (outfd) {
close(pipeout[1]);
-if(non_block)
-if(virSetNonBlock(pipeout[0]) == -1)
-ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-_(Failed to set non-blocking file descriptor
flag));
-
-if(virSetCloseExec(pipeout[0]) == -1)
-ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-_(Failed to set close-on-exec file descriptor
flag));
*outfd = pipeout[0];
}
if (errfd) {
close(pipeerr[1]);
-if(non_block)
-if(virSetNonBlock(pipeerr[0]) == -1)
-ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
- _(Failed to set non-blocking file descriptor
flag));
-
-if(virSetCloseExec(pipeerr[0]) == -1)
-ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
-_(Failed to set close-on-exec file descriptor
flag));
*errfd = pipeerr[0];
}
*retpid = pid;
+
+/* Restore our original signal mask now child is safely
+ running */
+if (pthread_sigmask(SIG_SETMASK, oldmask, NULL) 0) {
+ReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR,
+_(cannot unblock signals: %s),
+strerror(errno));
+return -1;
+}
+
return 0;
}
/* child */
+
+/* Clear out all signal handlers from parent so nothing
+ unexpected can happen in our child here */
+sig_action.sa_handler = SIG_DFL;
+sig_action.sa_flags = 0;
+sigemptyset(sig_action.sa_mask);
+
+for (i = 0 ; i NSIG ; i++)
I'd start at 1, since afaik, sigaction(0, ... serves no purpose.
+/* Only possible errors are EFAULT or EINVAL
+ The former wont happen, the latter we
+ expect, so no need to check return value */
+sigaction(i, sig_action, NULL);
+
+/* Unmask all signals in child, since we've no idea
+ what the caller's done with their signal mask */
+
[libvirt] [PATCH] static ip address
On Tue, Aug 12, 2008 at 06:49:59PM +0200, Olivier Deckmyn wrote:
On Tue, Aug 12, 2008 at 5:17 PM, Daniel Veillard [EMAIL PROTECTED]wrote:
That said I'm working on a new version of the patch which would be adequate
[...]
Once again, if I can do anything to help, I will.
Then please try the following patch, tell me if it works for you, I didn't
really tried it yet and parallel testing would help.
The things to test are:
- no crash
- the network definition is correctly parsed
- virsh network define and dump still work
- and IP as provided by dnsmasq as suggested
in case of mismatches try to grab the full command line used by libvirtd
for dnsmasq and report it in parallel of the XML file used,
thanks in advance,
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
[EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Index: src/network_conf.c
===
RCS file: /data/cvs/libxen/src/network_conf.c,v
retrieving revision 1.6
diff -u -r1.6 network_conf.c
--- src/network_conf.c 12 Aug 2008 08:25:48 - 1.6
+++ src/network_conf.c 12 Aug 2008 19:32:48 -
@@ -40,6 +40,7 @@
#include uuid.h
#include util.h
#include buf.h
+#include c-ctype.h
VIR_ENUM_DECL(virNetworkForward)
@@ -115,6 +116,13 @@
}
VIR_FREE(def-ranges);
+for (i = 0 ; i def-nhosts def-hosts ; i++) {
+VIR_FREE(def-hosts[i].mac);
+VIR_FREE(def-hosts[i].ip);
+VIR_FREE(def-hosts[i].host);
+}
+VIR_FREE(def-hosts);
+
VIR_FREE(def);
}
@@ -197,33 +205,82 @@
cur = node-children;
while (cur != NULL) {
-xmlChar *start, *end;
-
-if (cur-type != XML_ELEMENT_NODE ||
-!xmlStrEqual(cur-name, BAD_CAST range)) {
-cur = cur-next;
-continue;
-}
-
-if (!(start = xmlGetProp(cur, BAD_CAST start))) {
-cur = cur-next;
-continue;
-}
-if (!(end = xmlGetProp(cur, BAD_CAST end))) {
-cur = cur-next;
-xmlFree(start);
-continue;
-}
+if (cur-type == XML_ELEMENT_NODE
+xmlStrEqual(cur-name, BAD_CAST range)) {
+xmlChar *start, *end;
+
+if (!(start = xmlGetProp(cur, BAD_CAST start))) {
+cur = cur-next;
+continue;
+}
+if (!(end = xmlGetProp(cur, BAD_CAST end))) {
+cur = cur-next;
+xmlFree(start);
+continue;
+}
-if (VIR_REALLOC_N(def-ranges, def-nranges + 1) 0) {
-xmlFree(start);
-xmlFree(end);
-virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
-return -1;
+if (VIR_REALLOC_N(def-ranges, def-nranges + 1) 0) {
+xmlFree(start);
+xmlFree(end);
+virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+return -1;
+}
+def-ranges[def-nranges].start = (char *)start;
+def-ranges[def-nranges].end = (char *)end;
+def-nranges++;
+} else if (cur-type == XML_ELEMENT_NODE
+xmlStrEqual(cur-name, BAD_CAST hosts)) {
+xmlChar *mac, *host, *ip;
+unsigned char addr[6];
+struct in_addr inaddress;
+
+mac = xmlGetProp(cur, BAD_CAST mac);
+if ((mac != NULL)
+(virParseMacAddr((const char *) mac, addr[0]) != 0)) {
+virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot parse MAC address '%s'),
+ mac);
+VIR_FREE(mac);
+}
+host = xmlGetProp(cur, BAD_CAST host);
+if ((host != NULL) (!c_isalpha(host[0]))) {
+virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot use host address '%s'),
+ host);
+VIR_FREE(host);
+}
+/*
+ * You need at least one MAC address or one host name
+ */
+if ((mac == NULL) (host == NULL)) {
+VIR_FREE(mac);
+VIR_FREE(host);
+cur = cur-next;
+continue;
+}
+ip = xmlGetProp(cur, BAD_CAST ip);
+if (inet_pton(AF_INET, (const char *) ip, inaddress) = 0) {
+virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot parse IP address '%s'),
+ ip);
+VIR_FREE(ip);
+VIR_FREE(mac);
+VIR_FREE(host);
+cur =
Re: [libvirt] [PATCH] static ip address
On Tue, Aug 12, 2008 at 03:39:28PM -0400, Daniel Veillard wrote: On Tue, Aug 12, 2008 at 06:49:59PM +0200, Olivier Deckmyn wrote: On Tue, Aug 12, 2008 at 5:17 PM, Daniel Veillard [EMAIL PROTECTED]wrote: That said I'm working on a new version of the patch which would be adequate [...] Once again, if I can do anything to help, I will. Then please try the following patch, tell me if it works for you, I didn't really tried it yet and parallel testing would help. The things to test are: - no crash - the network definition is correctly parsed - virsh network define and dump still work - and IP as provided by dnsmasq as suggested in case of mismatches try to grab the full command line used by libvirtd for dnsmasq and report it in parallel of the XML file used, okay first bug, I changed the format from statichost mac=00:16:3E:XX:XX:XX host=XXX ip=192.168.122.2 / to host mac=00:16:3E:XX:XX:XX host=XXX ip=192.168.122.2 / but made a typo and it parses as hosts ... Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ [EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/ -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] static ip address
On Tue, Aug 12, 2008 at 03:39:28PM -0400, Daniel Veillard wrote:
On Tue, Aug 12, 2008 at 06:49:59PM +0200, Olivier Deckmyn wrote:
On Tue, Aug 12, 2008 at 5:17 PM, Daniel Veillard [EMAIL PROTECTED]wrote:
That said I'm working on a new version of the patch which would be
adequate
[...]
Once again, if I can do anything to help, I will.
Then please try the following patch, tell me if it works for you, I didn't
really tried it yet and parallel testing would help.
The things to test are:
- no crash
- the network definition is correctly parsed
- virsh network define and dump still work
- and IP as provided by dnsmasq as suggested
in case of mismatches try to grab the full command line used by libvirtd
for dnsmasq and report it in parallel of the XML file used,
New patch with just a pair of minor fixes which seems to work just fine for
me. The syntax is nearly the old one, just use host instead of statichost:
ip address=192.168.122.1 netmask=255.255.255.0
dhcp
range start=192.168.122.4 end=192.168.122.254 /
host mac=00:16:3E:XX:XX:XX host=XXX
ip=192.168.122.2 /
host host=YYY ip=192.168.122.3 /
/dhcp
/ip
I tried net-edit, net-xmldump, the dnsmasq arguments seems to be properly
set, please report,
thanks
Daniel
--
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard | virtualization library http://libvirt.org/
[EMAIL PROTECTED] | libxml GNOME XML XSLT toolkit http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
Index: src/network_conf.c
===
RCS file: /data/cvs/libxen/src/network_conf.c,v
retrieving revision 1.6
diff -u -r1.6 network_conf.c
--- src/network_conf.c 12 Aug 2008 08:25:48 - 1.6
+++ src/network_conf.c 12 Aug 2008 21:23:30 -
@@ -40,6 +40,7 @@
#include uuid.h
#include util.h
#include buf.h
+#include c-ctype.h
VIR_ENUM_DECL(virNetworkForward)
@@ -115,6 +116,13 @@
}
VIR_FREE(def-ranges);
+for (i = 0 ; i def-nhosts def-hosts ; i++) {
+VIR_FREE(def-hosts[i].mac);
+VIR_FREE(def-hosts[i].ip);
+VIR_FREE(def-hosts[i].host);
+}
+VIR_FREE(def-hosts);
+
VIR_FREE(def);
}
@@ -197,33 +205,82 @@
cur = node-children;
while (cur != NULL) {
-xmlChar *start, *end;
-
-if (cur-type != XML_ELEMENT_NODE ||
-!xmlStrEqual(cur-name, BAD_CAST range)) {
-cur = cur-next;
-continue;
-}
-
-if (!(start = xmlGetProp(cur, BAD_CAST start))) {
-cur = cur-next;
-continue;
-}
-if (!(end = xmlGetProp(cur, BAD_CAST end))) {
-cur = cur-next;
-xmlFree(start);
-continue;
-}
+if (cur-type == XML_ELEMENT_NODE
+xmlStrEqual(cur-name, BAD_CAST range)) {
+xmlChar *start, *end;
+
+if (!(start = xmlGetProp(cur, BAD_CAST start))) {
+cur = cur-next;
+continue;
+}
+if (!(end = xmlGetProp(cur, BAD_CAST end))) {
+cur = cur-next;
+xmlFree(start);
+continue;
+}
-if (VIR_REALLOC_N(def-ranges, def-nranges + 1) 0) {
-xmlFree(start);
-xmlFree(end);
-virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
-return -1;
+if (VIR_REALLOC_N(def-ranges, def-nranges + 1) 0) {
+xmlFree(start);
+xmlFree(end);
+virNetworkReportError(conn, VIR_ERR_NO_MEMORY, NULL);
+return -1;
+}
+def-ranges[def-nranges].start = (char *)start;
+def-ranges[def-nranges].end = (char *)end;
+def-nranges++;
+} else if (cur-type == XML_ELEMENT_NODE
+xmlStrEqual(cur-name, BAD_CAST host)) {
+xmlChar *mac, *host, *ip;
+unsigned char addr[6];
+struct in_addr inaddress;
+
+mac = xmlGetProp(cur, BAD_CAST mac);
+if ((mac != NULL)
+(virParseMacAddr((const char *) mac, addr[0]) != 0)) {
+virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot parse MAC address '%s'),
+ mac);
+VIR_FREE(mac);
+}
+host = xmlGetProp(cur, BAD_CAST host);
+if ((host != NULL) (!c_isalpha(host[0]))) {
+virNetworkReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot use host address '%s'),
+ host);
+VIR_FREE(host);
+}
+/*
+ * You need at least one MAC address or one host name
+ */
+if ((mac == NULL) (host == NULL)) {
+
Re: [libvirt] [PATCH]: ruby-libvirt migration fixes
On Fri, 2008-08-08 at 16:22 +0200, Chris Lalancette wrote:
Jim Meyering wrote:
diff -r c6a3e36cdf54 ext/libvirt/_libvirt.c
--- a/ext/libvirt/_libvirt.c Thu Jul 17 15:24:26 2008 -0700
+++ b/ext/libvirt/_libvirt.c Fri Aug 08 06:04:56 2008 -0400
@@ -637,16 +637,51 @@ VALUE libvirt_conn_num_of_defined_storag
}
#endif
+static char *get_string_or_nil(VALUE arg)
+{
+if (TYPE(arg) == T_NIL)
+return NULL;
+else if (TYPE(arg) == T_STRING)
+return STR2CSTR(arg);
STR2CSTR is marked as obsolete in ruby.h, where it says
to use StringValuePtr instead:
/* obsolete API - use StringValuePtr() */
#define STR2CSTR(x) rb_str2cstr((VALUE)(x),0)
Yeah, you are right. I looked through the ruby source code, actually, and I
ended up using StringValueCStr (which is used elsewhere in the ruby-libvirt
bindings). It's basically the same as StringValuePtr, but does an additional
check to make sure the string is not of 0 length and that there aren't
additional embedded \0 in the string.
I also updated the patch with the const pointers as you suggested. Attached.
Thanks for the review!
ACK .. committed.
I'll try and roll a new release tomorrow.
David
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] Question about html file in docs
Hi, I think html files in docs directory are redundunt. It is because html file is created by html.in. May I remove these files? Or are there any reason about staying these files? Thanks Atsushi SAKAI -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Implement vol delete for disk pools
Daniel P. Berrange wrote:
On Mon, Aug 11, 2008 at 03:58:41PM -0400, Cole Robinson wrote:
The patch below implements virStorageVolDelete for volumes
on a disk pool.
The only interesting thing here is that parted wants a
partition number to delete, so we need to peel off the
end of the volume's target path which will be of the form
'/dev/sda1' or similar (I assume. If not, it's still
better than having nothing implemented).
Thanks,
Cole
{
-/* delete a partition */
-virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
- _(Disk pools are not yet supported));
-return -1;
+char *part_num = NULL;
+int i;
+
+/* Strip target path (ex. '/dev/sda1') of its partition number */
+for (i = (strlen(vol-target.path)-1); i = 0; --i) {
+if (!c_isdigit(vol-target.path[i]))
+break;
+part_num = (vol-target.path[i]);
+}
+
+if (!part_num) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot parse partition number from target
+path '%s'), vol-target.path);
+return -1;
+}
This isn't correct because the target path is not guarenteed to point to
the master device name /dev/sda1. The user could have configured it to
use a stable path such as
/dev/disk/by-uuid/4cb23887-0d02-4e4c-bc95-7599c85afc1a
Hmm, I couldn't actually get /dev/disk/by-uuid to work. Seems like the
vol populating code for disks doesn't take into account the the pools
target path, and just uses the real partition path.
So, you need to first call 'readlink' on the vol-target.path, ignoring
EINVAL which you'll get if it wasn't a symlink. Once you've done that
you'll need to validate that it is sane by checking that
vol-source.devices[0]
is a prefix of the resolved pathname. Finally, you can extract the partition
number. So something along the lines of
char devname[PATH_MAX];
if (readlink(vol-target.path, devname, sizeof(devname)) 0
errno != EINVAL)
virStorageReportError(...)
if (!STRPREFIX(devname, vol-source.devices[0].path))
virStorageReportError()
part_num = devname + strlen(vol-source.devices[0].path)
The attached patch uses this approach. It works for the case with
vols of the form /dev/sdx123, but as mentioned above I couldn't
get by-uuid method to work, so can't be certain about that.
Thanks,
Cole
diff --git a/src/storage_backend_disk.c b/src/storage_backend_disk.c
index 31a35b5..889ed66 100644
--- a/src/storage_backend_disk.c
+++ b/src/storage_backend_disk.c
@@ -22,6 +22,7 @@
*/
#include config.h
+#include string.h
#include internal.h
#include storage_backend_disk.h
@@ -417,13 +418,6 @@ virStorageBackendDiskBuildPool(virConnectPtr conn,
return 0;
}
-
-static int
-virStorageBackendDiskDeleteVol(virConnectPtr conn,
- virStoragePoolObjPtr pool,
- virStorageVolDefPtr vol,
- unsigned int flags);
-
static int
virStorageBackendDiskCreateVol(virConnectPtr conn,
virStoragePoolObjPtr pool,
@@ -487,14 +481,56 @@ virStorageBackendDiskCreateVol(virConnectPtr conn,
static int
virStorageBackendDiskDeleteVol(virConnectPtr conn,
- virStoragePoolObjPtr pool ATTRIBUTE_UNUSED,
- virStorageVolDefPtr vol ATTRIBUTE_UNUSED,
+ virStoragePoolObjPtr pool,
+ virStorageVolDefPtr vol,
unsigned int flags ATTRIBUTE_UNUSED)
{
-/* delete a partition */
-virStorageReportError(conn, VIR_ERR_NO_SUPPORT,
- _(Disk pools are not yet supported));
-return -1;
+char *part_num = NULL;
+int i, n;
+char devname[PATH_MAX];
+
+if ((n = readlink(vol-target.path, devname, sizeof(devname))) 0
+errno != EINVAL) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(Couldn't read volume target path '%s'. %s),
+ vol-target.path, strerror(errno));
+return -1;
+} else if (n = 0) {
+strncpy(devname, vol-target.path, PATH_MAX);
+} else {
+devname[n] = '\0';
+}
+
+if (!STRPREFIX(devname, pool-def-source.devices[0].path)) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(Volume path did not start with parent pool
+path.));
+return -1;
+}
+
+part_num = devname + strlen(pool-def-source.devices[0].path);
+
+if (!part_num) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _(cannot parse partition number from target
+path '%s'), vol-target.path);
+return
Re: [libvirt] [PATCH] Change disk type 'dos' to 'msdos'
Daniel Veillard wrote: On Tue, Aug 12, 2008 at 11:12:07AM +0100, Daniel P. Berrange wrote: On Mon, Aug 11, 2008 at 10:01:16AM -0400, Cole Robinson wrote: Jim Meyering wrote: Cole Robinson [EMAIL PROTECTED] wrote: parted doesn't seem to want the label 'dos', instead wanting 'msdos'. Patch is basically s/dos/msdos/ Hi Cole, This definitely needs to be fixed. FYI, dos appears to be the preferred name for that partition table type (google for partition table and either dos or msdos; also, partx --type accepts dos, not msdos). Too bad Parted added the ms prefix. If it's not hard to implement, it would be nice to hide the implementation detail that Parted happens to call it the msdos label type. We could just keep the outward facing label the same, and internally map it to use msdos if calling 'parted mklabel' since it's the only command that seems to use that value. Whatever people think is best. Yep, I'd prefer that we just special case it internally when calling out to mklabel. Fine by me, my only worry is that we are somehow breaking the storage XML format as a result, but I don't think this is widely used at this point (at least with MSDos) and best done earlier than later, Applied and commited, thanks ! Daniel Wait, I think there was a bit of miscommunication. I think Jim and Dan were recommending a different approach then the patch that was committed. The attached patch keeps the public facing formats the same, we just special case the dos format to use msdos if calling out to parted (this would require the current patch to be reverted though). Dan, is this what you meant? Thanks, Cole commit 3d32ee59f405c6fb38b8ec01813804387f237a21 Author: Cole (Work Acct) [EMAIL PROTECTED] Date: Tue Aug 12 21:29:58 2008 -0400 Use dos disk pool type when calling parted mklabel. diff --git a/src/storage_backend_disk.c b/src/storage_backend_disk.c index dac827b..31a35b5 100644 --- a/src/storage_backend_disk.c +++ b/src/storage_backend_disk.c @@ -406,7 +406,8 @@ virStorageBackendDiskBuildPool(virConnectPtr conn, PARTED, pool-def-source.devices[0].path, mklabel, -virStorageBackendDiskPoolFormatToString(conn, pool-def-source.format), +((pool-def-source.format == VIR_STORAGE_POOL_DISK_DOS) ? msdos : + virStorageBackendDiskPoolFormatToString(conn, pool-def-source.format)), NULL, }; -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Fix size reporting for disk pools without partitions
The attached patch updates parthelper to print size
information for a disk device if it doesn't have any
allocated partitions.
The current code starts by requesting the first partition,
then iterating from there. But if there is no first
partition, that whole info reporting thing never happens :)
Seems to produce desired results for partitioned and
unpartitioned devices.
Thanks,
Cole
commit 4a52468112e698e6f3f7ba94141d1e315579d3cd
Author: Cole (Work Acct) [EMAIL PROTECTED]
Date: Tue Aug 12 21:28:46 2008 -0400
Fix size reporting for disks without partitions.
diff --git a/src/parthelper.c b/src/parthelper.c
index 1cd7240..c699035 100644
--- a/src/parthelper.c
+++ b/src/parthelper.c
@@ -63,7 +63,7 @@ int main(int argc, char **argv)
}
/* Get the first partition, and then iterate over all */
-part = ped_disk_get_partition(disk, 1);
+part = ped_disk_next_partition(disk, NULL);
while (part) {
const char *type;
const char *content;
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] Ignore specified target path when creating logical vol
Specifying a target path when creating a storage
volume has no effect, since volumes only really use
the pool's target path and 'name' field to
establish the volume's target. Logical volumes
expect a target path to be passed, and it can
only cause problems.
The attached patch erases the passed target volume
and fills it in with the one generated from the
create operation.
Thanks,
Cole
commit 3e2a4e2fa460a34330a08ab77fe67bcbd542f519
Author: Cole (Work Acct) [EMAIL PROTECTED]
Date: Tue Aug 12 21:55:54 2008 -0400
Fix creating and cleaning up logical volumes if a target path is specified.
diff --git a/src/storage_backend_logical.c b/src/storage_backend_logical.c
index 0c4f6a5..eb362c6 100644
--- a/src/storage_backend_logical.c
+++ b/src/storage_backend_logical.c
@@ -453,6 +453,19 @@ virStorageBackendLogicalCreateVol(virConnectPtr conn,
snprintf(size, sizeof(size)-1, %lluK, vol-capacity/1024);
size[sizeof(size)-1] = '\0';
+if (vol-target.path != NULL) {
+/* A target path passed to CreateVol has no meaning */
+VIR_FREE(vol-target.path);
+}
+if (VIR_ALLOC_N(vol-target.path, strlen(pool-def-target.path) +
+1 + strlen(vol-name) + 1) 0) {
+virStorageReportError(conn, VIR_ERR_NO_MEMORY, %s, _(volume));
+return -1;
+}
+strcpy(vol-target.path, pool-def-target.path);
+strcat(vol-target.path, /);
+strcat(vol-target.path, vol-name);
+
if (virRun(conn, cmdargv, NULL) 0)
return -1;
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
