Re: [libvirt] [PATCH] Avoid unnecessary bootstrap runs in VPATH builds
OK, this should be the complete fix. I like your option better. Jirka autogen.sh |2 +- cfg.mk |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) ACK to this version. OK, thanks. I pushed it now. Jirka -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] nodeinfo: skip offline CPUs
On Tue, Aug 10, 2010 at 04:17:56PM -0600, Eric Blake wrote: On 08/10/2010 04:01 PM, Daniel Veillard wrote: On Tue, Aug 10, 2010 at 03:44:37PM -0600, Eric Blake wrote: https://bugzilla.redhat.com/622515 - When hot-unplugging CPUs, libvirt failed to start a guest that had been pinned to CPUs that were still online, because it was failing to read status from unrelated offline CPUs. Argh, yes that's a nasty problem ! ACK, that looks fine ! thanks ! Thanks for the review. I'm squashing this in before applying, based on an IRC comment by Dave Allan that 1024 is an awfully big stack allocation when we know that the value will fit in an int. diff --git i/src/nodeinfo.c w/src/nodeinfo.c index 2a20679..59e0163 100644 --- i/src/nodeinfo.c +++ w/src/nodeinfo.c @@ -48,6 +48,7 @@ #include logging.h #include virterror_internal.h #include count-one-bits.h +#include intprops.h #define VIR_FROM_THIS VIR_FROM_NONE @@ -72,7 +73,7 @@ static int get_cpu_value(unsigned int cpu, const char *file, bool missing_ok) { char *path; FILE *pathfp; -char value_str[1024]; +char value_str[INT_BUFSIZE_BOUND(int)]; char *tmp; int value = -1; Whoops I didn't catch that ! Actually I wonder if we could catch sequences like \[[0-9]..[0-9]+\] in make syntax-check, maybe this could make sure we always dynamically allocate such large buffers. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] allow memballoon type of none to desactivate it
On Mon, Aug 09, 2010 at 06:53:34PM +0100, Daniel P. Berrange wrote: On Mon, Aug 09, 2010 at 06:38:27PM +0200, Daniel Veillard wrote: The balloon device is automatically added to qemu guests if supported, but it may be useful to desactivate it. The simplest to not change the existing behaviour is to allow memballoon type=none/ as an extra option to desactivate it (it is automatically added if the memballoon construct is missing for the domain). The following simple patch just adds the extra option and does not change the default behaviour but avoid creating a balloon device if type=none is used. I really don't like the idea of 'type=none' devices in general. I don't think we should have an element insides devices that doesn't actually represent a device. If we want to disable the balloon, then I think we should aim for an element or attribute elsewhere to toggle it. eg, perhaps the earlier memory element can indicate whether it supports ballooning. eg memory ballonable='yes|no'2423423432/memory Thus if ballooning is not enabled, the memballoon device would never need to appear within devices Okay being stuck and having though about the issue for a couple of days I think memballoon type=none/ is the most pragmatic way to avoid forcing the memballoon device on QEmu/KVM users at this point. The issue in general of memory configuration is somehow a mess as you pointed out, there is 4 tunables or constructs appaering to control them, but getting to a cleaner way to manage this may take some time. I'm taking the responsability of adding that extra enum in the list and the single line change in the qemu code (the other change is just a comment), and whatever solution we may end up with I think it will be trivial to detect the enum value and switch it on the fly. From a device management perspective since there can be only 1 memballoon device per guest I think handling this issue should be trivial there too. So I commited that vary small patch and will take the burden of making the conversion to whatever construct we may end up picking on the long term for this setting. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Explicitly disable unused floppy0 device for GSX
On Mon, Aug 09, 2010 at 04:44:14PM -0600, Eric Blake wrote: On 08/05/2010 06:45 PM, Matthias Bolte wrote: floppy0.present defaults to true for GSX. Therefore, it needs to be explicitly disabled for GSX. --- src/esx/esx_vmx.c | 10 ++ 1 files changed, 10 insertions(+), 0 deletions(-) If I understand correctly, you want to uniformly make floppy not present except when the user asked for it. The other ESX clients meet this goal, but for GSX, you have to add an extra argument to make it so. That being the case, I agree with the approach. @@ -2539,6 +2544,11 @@ esxVMX_FormatConfig(esxVMX_Context *ctx, virCapsPtr caps, virDomainDefPtr def, } } +if (!hasFloppyDevice productVersion esxVI_ProductVersion_GSX) { I'd add an extra () here to make precedence clear: if (!hasFloppyDevice (productVersion esxVI_ProductVersion_GSX)) { ACK with that nit fixed. Agreed, ACK Matthias you push it ? Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Block-migrate
On Mon, Aug 09, 2010 at 11:57:34PM +0200, Ruben Kerkhof wrote: Hi Justin, On Mon, Aug 9, 2010 at 22:30, Justin Clift jcl...@redhat.com wrote: On 08/07/2010 11:55 PM, Ruben Kerkhof wrote: snip Maybe I'm using the wrong options for virsh migrate, but they're not described in the manpage. Any hints? Hi Ruben, Haven't tried this stuff myself yet, but the error you're mentioning look similar to what can happen if SELinux is enabled and blocking things. Same for AppArmor too I guess. Is there any chance that might be the problem? Selinux is running in permissive mode, and I don't see any related AVC's in my audit log. I'm on Fedora 13 by the way, using libvirt 0.8.3. I've tried this with both qemu-kvm 0.12.3 and a git shapshot of the upcoming qemu-kvm 0.13. Is the error lso showing up if you're using a shared storage for migration ? A couple of things to look for are DNS entries, make sure both source and target are in the DNS and resolve to their IP addresses, also check iptables ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Improve VMX file name parsing and formatting
On Sat, Aug 07, 2010 at 09:54:34PM +0200, Matthias Bolte wrote: 2010/8/7 Matthias Bolte matthias.bo...@googlemail.com: For parsing try to match by datastore mount path first, if that fails fallback to /vmfs/volumes/datastore/path parsing. This also fixes problems with GSX on Windows. Because GSX on Windows doesn't use /vmfs/volumes/ style file names. For formatting use the datastore mount path too, instead of using /vmfs/volumes/datastore/path as fixed format. --- src/esx/esx_driver.c | 372 +++-- 1 files changed, 235 insertions(+), 137 deletions(-) + + /* Strip trailing separators */ + length = strlen(hostMount-mountInfo-path); + + if (length 0) { + tmp = hostMount-mountInfo-path + length - 1; + + while (*tmp == separator tmp hostMount-mountInfo-path) { + --tmp; } [skip removed code] + length = tmp - hostMount-mountInfo-path; + } + + /* Format as mount[/directory]/file */ + virBufferAdd(buffer, hostMount-mountInfo-path, length); + The trailing separators stripping it totally broken :( This incremental diff fixes that: diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 322c588..4fb357b 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -286,14 +286,8 @@ esxFormatVMXFileName(const char *datastorePath, void *opaque) /* Strip trailing separators */ length = strlen(hostMount-mountInfo-path); -if (length 0) { -tmp = hostMount-mountInfo-path + length - 1; - -while (*tmp == separator tmp hostMount-mountInfo-path) { ---tmp; -} - -length = tmp - hostMount-mountInfo-path; +while (length 0 hostMount-mountInfo-path[length - 1] == separator) { +--length; } /* Format as mount[/directory]/file */ Okay, ACK :-) Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ dan...@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Block-migrate
On Sat, Aug 07, 2010 at 03:55:10PM +0200, Ruben Kerkhof wrote: I've been playing with the new block-migrate feature, but am unable to get it to work. [r...@src ~]# virsh migrate --live --p2p --tunnelled --copy-storage-all 4c5c75b9-decc-41c9-9296-20ca5bd5c355 qemu://dst/system error: Unknown failure /var/log/libvirt/qemu/4c5c75b9-decc-41c9-9296-20ca5bd5c355.log on the destination host shows: bind(unix:/var/run/libvirt/qemu/qemu.tunnelmigrate.dest.4c5c75b9-decc-41c9-9296-20ca5bd5c355): Permission denied Migration failed. Exit code unix:/var/run/libvirt/qemu/qemu.tunnelmigrate.dest.4c5c75b9-decc-41c9-9296-20ca5bd5c355(-22), exiting It seems that qemu is not able to write to that location. [r...@dst qemu]# ls -ld /var/run/libvirt/qemu/ drwx--. 2 root root 4096 Aug 7 15:40 /var/run/libvirt/qemu/ As a workaround I gave qemu write permission, and now the block migrate starts. Did the disk image files exist on the target before migration started ? If not, then this is going to be falling foul of our security drivers which prevent QEMU creating new files itself. Also, a straw poll of people at the KVM forum yesterday determined that no one had ever got block migration to work, so it might be just broken in QEMU Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Block-migrate
Hi Daniel, On Wed, Aug 11, 2010 at 14:16, Daniel P. Berrange berra...@redhat.com wrote: On Sat, Aug 07, 2010 at 03:55:10PM +0200, Ruben Kerkhof wrote: I've been playing with the new block-migrate feature, but am unable to get it to work. [r...@src ~]# virsh migrate --live --p2p --tunnelled --copy-storage-all 4c5c75b9-decc-41c9-9296-20ca5bd5c355 qemu://dst/system error: Unknown failure /var/log/libvirt/qemu/4c5c75b9-decc-41c9-9296-20ca5bd5c355.log on the destination host shows: bind(unix:/var/run/libvirt/qemu/qemu.tunnelmigrate.dest.4c5c75b9-decc-41c9-9296-20ca5bd5c355): Permission denied Migration failed. Exit code unix:/var/run/libvirt/qemu/qemu.tunnelmigrate.dest.4c5c75b9-decc-41c9-9296-20ca5bd5c355(-22), exiting It seems that qemu is not able to write to that location. [r...@dst qemu]# ls -ld /var/run/libvirt/qemu/ drwx--. 2 root root 4096 Aug 7 15:40 /var/run/libvirt/qemu/ As a workaround I gave qemu write permission, and now the block migrate starts. Did the disk image files exist on the target before migration started ? If not, then this is going to be falling foul of our security drivers which prevent QEMU creating new files itself. I'm working with raw lvs and created them on the destination before starting the migration. Also, a straw poll of people at the KVM forum yesterday determined that no one had ever got block migration to work, so it might be just broken in QEMU Daniel Actually, I've got it working now, with the --direct option and the --p2p --direct option. The tunneled method fails though. I'm trying to debug this now. The first thing which strikes me as odd is in the qemuMonitorTextMigrate function: 1173if (qemuMonitorCommand(mon, cmd, info) 0) { (gdb) p cmd $4 = 0x7fffdc0e6ef0 migrate -d -b\unix:/var/run/libvirt/qemu/qemu.tunnelmigrate.src.4c5c75b9-decc-41c9-9296-20ca5bd5c355\ (gdb) n 1180if (strstr(info, fail) != NULL) { (gdb) n 1181qemuReportError(VIR_ERR_OPERATION_FAILED, (gdb) p info $5 = 0x7fffe8001210 migration failed\r\n Not sure if there's supposed to be a space between the migrate and the -d option, and no space after the -b. Regards, Ruben -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Block-migrate
On Wed, Aug 11, 2010 at 14:22, Ruben Kerkhof ru...@rubenkerkhof.com wrote: Actually, I've got it working now, with the --direct option and the --p2p --direct option. The tunneled method fails though. I'm trying to debug this now. I've got the tunnelled method working now as well, but had to disable the security driver on the destination host, and run qemu as root/root. Regards, Ruben -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Split VMX code into a general and an ESX specific part
2010/8/10 Eric Blake ebl...@redhat.com: On 08/05/2010 09:47 AM, Matthias Bolte wrote: Introduce esxVMX_Context containing functions pointers to glue both parts together in an generic way. s/an/a/ Fixed. Move the ESX specific part to esx_driver.c. This is a step towards making the VMX code reusable in a potential VMware Workstation and VMware Player driver. --- src/esx/esx_driver.c | 417 ++- src/esx/esx_vmx.c | 448 +- src/esx/esx_vmx.h | 71 + tests/vmx2xmltest.c | 52 ++- tests/xml2vmxtest.c | 64 +++- 5 files changed, 611 insertions(+), 441 deletions(-) Looks like the bulk of the patch is code motion and test addition. ACK. Thanks, and pushed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] Add ubd to the list of disk prefixes
On Tue, Aug 10, 2010 at 07:18:06AM -0600, Eric Blake wrote: Pushed, along with a .mailmap update to satisfy 'make syntax-check'. Soren, this patch introduced a third distinct author address for you - let me know if you want AUTHORS updated to swap which one is listed as your primary address. so...@canonical.com is no longer valid, so I won't be submitting any more patches from that address. Let's go with so...@linux2go.dk as the primary one and leave so...@ubuntu.com as an alias. -- Soren Hansen so...@linux2go.dk Systems Architect, The Rackspace Cloud Ubuntu Developer -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/2] Add a detailed message when tap device add fails
On Tue, Aug 10, 2010 at 5:38 PM, Eric Blake ebl...@redhat.com wrote: On 08/10/2010 04:14 PM, Eric Blake wrote: On 08/10/2010 03:43 PM, Doug Goldstein wrote: Added a more detailed error message when adding a tap devices fails and the kernel is missing tun support. Signed-off-by: Doug Goldstein car...@gentoo.org Both of these are already in git. http://libvirt.org/git/?p=libvirt.git;a=commit;h=0890a70a1 http://libvirt.org/git/?p=libvirt.git;a=commit;h=bcc8b58be Any reason for the resend? Whoops - my bad. This touches uml, rather than qemu (other than the difference in files, the patches look the same). ACK, applying them, and apologies for mis-reading them ;) Yeah I should have clarified sorry. Thanks for applying them. -- Doug Goldstein -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] using sync_manager with libvirt
Hi, We've been working on a program called sync_manager that implements shared-storage-based leases to protect shared resources. One way we'd like to use it is to protect vm images that reside on shared storage, i.e. preventing two vm's on two hosts from using the same image at once. It's functional, and the next big step is using it through libvirt. sync_manager wraps a process, i.e. acquires a lease, forksexecs a process, renews the lease wile the process runs, and releases the lease when the process exits. While the process runs, it has exclusive access to whatever resource was named in the lease that was acquired. A command would be something like this: sync_manager daemon -i host_id -n vm_id -l lease -c command args host_id is integer between 1 and 2000 that is statically assigned to each host. vm_id is a unique identifer for the process that will be run, e.g. the vm name or uuid. lease defines the shared storage area that sync_manager should use for performing the disk-paxos based synchronization. It consists of resource_name:path:offset, where resource_name is likely to be the vm name/uuid (or the name of the vm's disk image), and path:offset is an area of shared storage that has been allocated for sync_manager to use (a separate area for each resource/vm). command args would be the qemu command line that is currently used. We expect these new config values will need a place to live in the libvirt xml config file, and libvirt will need to fork sync_manager -c qemu rather than qemu directly. At least those are the most obvious things that need doing, there are sure to be other api or functional issues. sync_manager only forks when running the command, and doesn't change any fd's, so any command output should appear unchanged to libvirt. Would there be any problem with sync_manager also printing its own warnings and errors to stderr? While the main point of sync_manager is the disk leases to synchronize access among hosts, it also uses posix locks to synchronize access among local processes. http://git.fedorahosted.org/git/?p=sync_manager.git Thanks, Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Block-migrate
On 08/11/2010 11:35 PM, Ruben Kerkhof wrote: On Wed, Aug 11, 2010 at 14:22, Ruben Kerkhofru...@rubenkerkhof.com wrote: Actually, I've got it working now, with the --direct option and the --p2p --direct option. The tunneled method fails though. I'm trying to debug this now. I've got the tunnelled method working now as well, but had to disable the security driver on the destination host, and run qemu as root/root. Hi Ruben, Would you have time to add the steps / command line arguments you used to a page on the wiki? Writing it up in a fuller fashion would take some time, but it sounds like you have the steps for making it work understood, so even basic info to help people would be very cool. :) ? Regards and best wishes, Justin Clift -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] Block-migrate
On Wed, Aug 11, 2010 at 18:08, Justin Clift jcl...@redhat.com wrote: Would you have time to add the steps / command line arguments you used to a page on the wiki? Writing it up in a fuller fashion would take some time, but it sounds like you have the steps for making it work understood, so even basic info to help people would be very cool. :) Sure, I'll write something up, and see if I can come up with a patch for the virsh manpage as well. Regards, Ruben -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] bridge: Fix potential segfault when preparing dnsmasq arguments
We add --dhcp-lease-max=xxx argument when network-def-nranges 0 but we only allocate space for in the opposite case :-) I guess we are lucky enough to miscount somewhere else so that we actually allocate more space than we need since no-one has hit this bug so far. --- src/network/bridge_driver.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 59e02b1..f247a0f 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -426,7 +426,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, 2 + /* --listen-address 10.0.0.1 */ (2 * network-def-nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ /* --dhcp-lease-max=xxx if needed */ -(network-def-nranges ? 0 : 1) + +(network-def-nranges ? 1 : 0) + /* --dhcp-hostsfile=/var/lib/dnsmasq/$NAME.hostsfile */ (network-def-nhosts 0 ? 1 : 0) + /* --enable-tftp --tftp-root /srv/tftp */ -- 1.7.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] using sync_manager with libvirt
On Tue, Aug 10, 2010 at 12:44:06PM -0400, David Teigland wrote: Hi, We've been working on a program called sync_manager that implements shared-storage-based leases to protect shared resources. One way we'd like to use it is to protect vm images that reside on shared storage, i.e. preventing two vm's on two hosts from using the same image at once. There's two different, but related problems here: - Preventing 2 different VMs using the same disk - Preventing the same VM running on 2 hosts at once The first requires that there is a lease per configured disk (since a guest can have multiple disks). The latter requires a lease per VM and can ignore specifices of what disks are configured. IIUC, sync-manager is aiming for the latter. It's functional, and the next big step is using it through libvirt. sync_manager wraps a process, i.e. acquires a lease, forksexecs a process, renews the lease wile the process runs, and releases the lease when the process exits. While the process runs, it has exclusive access to whatever resource was named in the lease that was acquired. There are complications around migration we need to consider too. During migration, you actually need QEMU running on two hosts at once. IIRC the idea is that before starting the migration operation, we'd have to tell sync-manager to mark the lease as shared with a specific host. The destination QEMU would have to startup in shared mode, and upgrade this to an exclusive lock when migration completes, or quit when migration fails. For libvirt integration, this means we need to consider an approach that is much broader than just adding a wrapper process. A command would be something like this: sync_manager daemon -i host_id -n vm_id -l lease -c command args host_id is integer between 1 and 2000 that is statically assigned to each host. vm_id is a unique identifer for the process that will be run, e.g. the vm name or uuid. lease defines the shared storage area that sync_manager should use for performing the disk-paxos based synchronization. It consists of resource_name:path:offset, where resource_name is likely to be the vm name/uuid (or the name of the vm's disk image), and path:offset is an area of shared storage that has been allocated for sync_manager to use (a separate area for each resource/vm). Can you give some real examples of the lease arg ? I guess path must exclude the ':' character, or have some defined escaping scheme. command args would be the qemu command line that is currently used. We expect these new config values will need a place to live in the libvirt xml config file, and libvirt will need to fork sync_manager -c qemu rather than qemu directly. At least those are the most obvious things that need doing, there are sure to be other api or functional issues. The host_id is obviously needs to be in /etc/libvirt/sync-manager.conf since that's a per-host config. I assume the shared storage area is per host too ? That leaves just the VM name/uuid as a per-VM config option, and we obviously already have that in XML. Is there actually any extra attribute we need to track per-guest in the XML ? If not this will simplify life, because we won't have to track sync-manager specific attributes sync_manager only forks when running the command, and doesn't change any fd's, so any command output should appear unchanged to libvirt. Would there be any problem with sync_manager also printing its own warnings and errors to stderr? That's fine. Printing to stderr upon error is actually required. This is the only way we can get VM startup failure messages back up to the user. While the main point of sync_manager is the disk leases to synchronize access among hosts, it also uses posix locks to synchronize access among local processes. http://git.fedorahosted.org/git/?p=sync_manager.git No docs ? In terms of integration with libvirt, I think it is desirable that we keep libvirt and sync-manager loosely coupled. ie We don't want to hardcode libvirt using sync-manager, nor do we want to hardcode sync-manager only working with libvirt. This says to me that we need to provide a well defined plugin system for providing a 'supervisor process' for QEMU guests. Essentially a dlopen() module that provides a handful ( 10) callbacks which are triggered in appropriate codepaths. At minimum I expect we need - A callback at ARGV building, to let extra sync-manager ARGV to be injected - A callback at VM startup. Not needed for sync-manager, but to allowfor alternate impls that aren't based around supervising. - A callback at VM shutdown. Just to cleanup resources - A callback in the VM destroy method, in case we need todo something different other than just kill($PID) the QEMU $PID. (eg to perhaps tell sync-manager to kill QEMU
Re: [libvirt] [PATCH] bridge: Fix potential segfault when preparing dnsmasq arguments
On 08/11/2010 10:44 AM, Jiri Denemark wrote: We add --dhcp-lease-max=xxx argument when network-def-nranges 0 but we only allocate space for in the opposite case :-) I guess we are lucky enough to miscount somewhere else so that we actually allocate more space than we need since no-one has hit this bug so far. ACK. Boy will it be nice to move to the newer exec wrappers that eliminates the need for each client to malloc the right size argv themselves. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] bridge: Add --dhcp-no-override option to dnsmasq
--dhcp-no-override description from dnsmasq man page: Disable re-use of the DHCP servername and filename fields as extra option space. If it can, dnsmasq moves the boot server and filename information (from dhcp-boot) out of their dedicated fields into DHCP options. This make extra space available in the DHCP packet for options but can, rarely, confuse old or broken clients. This flag forces simple and safe behaviour to avoid problems in such a case. It seems some virtual network card ROMs are this old/buggy so let's add --dhcp-no-override as a workaround for them. We don't use extra DHCP options so this should be safe. --- src/network/bridge_driver.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index f247a0f..d6d3068 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -427,6 +427,8 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, (2 * network-def-nranges) + /* --dhcp-range 10.0.0.2,10.0.0.254 */ /* --dhcp-lease-max=xxx if needed */ (network-def-nranges ? 1 : 0) + +/* --dhcp-no-override if needed */ +(network-def-nranges ? 1 : 0) + /* --dhcp-hostsfile=/var/lib/dnsmasq/$NAME.hostsfile */ (network-def-nhosts 0 ? 1 : 0) + /* --enable-tftp --tftp-root /srv/tftp */ @@ -497,6 +499,7 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network, if (network-def-nranges 0) { snprintf(buf, sizeof(buf), --dhcp-lease-max=%d, nbleases); APPEND_ARG(*argv, i++, buf); +APPEND_ARG(*argv, i++, --dhcp-no-override); } if (network-def-nhosts 0) { -- 1.7.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] using sync_manager with libvirt
On Wed, Aug 11, 2010 at 05:59:55PM +0100, Daniel P. Berrange wrote: On Tue, Aug 10, 2010 at 12:44:06PM -0400, David Teigland wrote: Hi, We've been working on a program called sync_manager that implements shared-storage-based leases to protect shared resources. One way we'd like to use it is to protect vm images that reside on shared storage, i.e. preventing two vm's on two hosts from using the same image at once. There's two different, but related problems here: - Preventing 2 different VMs using the same disk - Preventing the same VM running on 2 hosts at once The first requires that there is a lease per configured disk (since a guest can have multiple disks). The latter requires a lease per VM and can ignore specifices of what disks are configured. IIUC, sync-manager is aiming for the latter. The present integration effort is aiming for the latter. sync_manager itself aims to be agnostic about what it's managing. It's functional, and the next big step is using it through libvirt. sync_manager wraps a process, i.e. acquires a lease, forksexecs a process, renews the lease wile the process runs, and releases the lease when the process exits. While the process runs, it has exclusive access to whatever resource was named in the lease that was acquired. There are complications around migration we need to consider too. During migration, you actually need QEMU running on two hosts at once. IIRC the idea is that before starting the migration operation, we'd have to tell sync-manager to mark the lease as shared with a specific host. The destination QEMU would have to startup in shared mode, and upgrade this to an exclusive lock when migration completes, or quit when migration fails. sync_manager leases can only be exclusive, so it's a matter of transfering ownership of the exclusive lock from source host to destination host. We have not yet added lease transfer capabilities to sync_manager, but it might look something like this: S = source host, sm-S = sync_manager on S, ... D = destination host, sm-D = sync_manager on D, ... 1. sm-S holds the lease, and is monitoring qemu 2. migration begins from S to D 3. libvirt-D runs sm-D: sync_manager -c qemu with the addition of a new sync_manager option --receive-lease 4. sm-D writes its hostid D to the lease area signaling sm-S that it wants to be the lease owner when S is done with it 5. sm-D begins monitoring the lease owner on disk (which is still S) 6. sm-D forks qemu-D 7. sm-S sees that D wants the lease 8. qemu-S exits with success 9. sm-S sees qemu-S exit with success 10. sm-S writes D as the lease owner into the lease area and exits (in the non-migration/transfer case, sm-S writes owner=LEASE_FREE) 11. sm-D (still monitoring the lease owner) sees that it has become the owner, and begins renewing the lease 12. qemu-D runs fully I don't know enough (anything) about qemu migration yet to say if those steps work correctly or safely. One concern is that qemu-D should not enter a state where it can write until we are certain that D has been written as the lease's owner. sync_manager daemon -i host_id -n vm_id -l lease -c command args lease defines the shared storage area that sync_manager should use for performing the disk-paxos based synchronization. It consists of resource_name:path:offset, where resource_name is likely to be the vm name/uuid (or the name of the vm's disk image), and path:offset is an area of shared storage that has been allocated for sync_manager to use (a separate area for each resource/vm). Can you give some real examples of the lease arg ? I guess path must exclude the ':' character, or have some defined escaping scheme. -l vm0:/dev/vg/lease_area:0 (exclude : from paths) Manually setting up, intializing and keeping track of lease areas would be a pain, so we'll definately be looking at adding that to higher level tools. The host_id is obviously needs to be in /etc/libvirt/sync-manager.conf since that's a per-host config. I assume the shared storage area is per host too ? That leaves just the VM name/uuid as a per-VM config option, and we obviously already have that in XML. Is there actually any extra attribute we need to track per-guest in the XML ? If not this will simplify life, because we won't have to track sync-manager specific attributes With the plugin style hooks you describe below, it seems all the sync_manager config could be kept separate from the libvirt config. In terms of integration with libvirt, I think it is desirable that we keep libvirt and sync-manager loosely coupled. ie We don't want to hardcode libvirt using sync-manager, nor do we want to hardcode sync-manager only working with libvirt. This says to me that we need to provide a well defined plugin system for providing a 'supervisor process' for QEMU guests. Essentially a
Re: [libvirt] [PATCH] bridge: Add --dhcp-no-override option to dnsmasq
On 08/11/2010 12:41 PM, Jiri Denemark wrote: --dhcp-no-override description from dnsmasq man page: Disable re-use of the DHCP servername and filename fields as extra option space. If it can, dnsmasq moves the boot server and filename information (from dhcp-boot) out of their dedicated fields into DHCP options. This make extra space available in the DHCP packet for options but can, rarely, confuse old or broken clients. This flag forces simple and safe behaviour to avoid problems in such a case. It seems some virtual network card ROMs are this old/buggy so let's add --dhcp-no-override as a workaround for them. We don't use extra DHCP options so this should be safe. --- src/network/bridge_driver.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) Are we guaranteed that --dhcp-no-override is supported as far back as the earliest version of dnsmasq that we claim to run with, or do we need to add some --help parsing? Conditional ACK if we are happy that all versions of dnsmasq that we care about can support this flag. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] using sync_manager with libvirt
On 08/11/2010 02:53 PM, Chris Lalancette wrote: Unfortunately, this is not how migration works in qemu/kvm. Using your nomenclature above, it's more like the following: A guest is running on S. A migration is then initiated, at which point D fires up a qemu process with a -incoming argument. This is sort of a container process that will receive all of the migration data. Crucially for sync-manager, though, qemu completely starts up and attaches to all of the resources (including disks) *while* qemu at S is still running. Then it enters a sort of paused state (where the guest cannot run), and receives all of the migration data. Once all of the migration data has been received, the guest on S is destroyed, and the guest on D is unpaused. That's why Dan mentioned that we need two hosts to access the disk at once. On the other hand, does D do any writes to the disk prior to the point at which it is unpaused? Would it work if D can be granted a read-only lease to access to the disk for the duration of the migration data, and then be converted over to read-write at the point when S is destroyed? On a related vein, libguestfs provides things like 'guestfish --ro', which is documented as a safe way to do read-only access of a disk image in use by another VM. That serves as another case where we want to be able to provide read-only access to a disk while someone else holds the read-write lease. -- Eric Blake ebl...@redhat.com+1-801-349-2682 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] using sync_manager with libvirt
On Wed, Aug 11, 2010 at 04:53:20PM -0400, Chris Lalancette wrote: 1. sm-S holds the lease, and is monitoring qemu 2. migration begins from S to D 3. libvirt-D runs sm-D: sync_manager -c qemu with the addition of a new sync_manager option --receive-lease 4. sm-D writes its hostid D to the lease area signaling sm-S that it wants to be the lease owner when S is done with it 5. sm-D begins monitoring the lease owner on disk (which is still S) 6. sm-D forks qemu-D 7. sm-S sees that D wants the lease 8. qemu-S exits with success 9. sm-S sees qemu-S exit with success 10. sm-S writes D as the lease owner into the lease area and exits (in the non-migration/transfer case, sm-S writes owner=LEASE_FREE) 11. sm-D (still monitoring the lease owner) sees that it has become the owner, and begins renewing the lease 12. qemu-D runs fully Unfortunately, this is not how migration works in qemu/kvm. Using your nomenclature above, it's more like the following: A guest is running on S. A migration is then initiated, at which point D fires up a qemu process with a -incoming argument. libvirt starts qemu -incoming on D, right? So with sync_manager, libvirt would start: sync_manager --receive_lease -c qemu -incoming This is sort of a container process that will receive all of the migration data. Crucially for sync-manager, though, qemu completely starts up and attaches to all of the resources (including disks) *while* qemu at S is still running. Then it enters a sort of paused state (where the guest cannot run), and receives all of the migration data. That should all be fine. Once all of the migration data has been received, the guest on S is destroyed, ok, sm-S sees qemu-S exit at that point. and the guest on D is unpaused. The critical bit would be ensuring that sm-S has written owner=D into the lease area before qemu-D is unpaused. Hooking into the sequence at that point in time might be too difficult or ugly, I don't know. That's why Dan mentioned that we need two hosts to access the disk at once. It would be easiest, of course, if the lease owner always represented where qemu was running, but that obviously won't work with migration. So we have to settle for the lease owner always representing where qemu is unpaused. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] using sync_manager with libvirt
On Wed, Aug 11, 2010 at 03:37:12PM -0400, David Teigland wrote: On Wed, Aug 11, 2010 at 05:59:55PM +0100, Daniel P. Berrange wrote: On Tue, Aug 10, 2010 at 12:44:06PM -0400, David Teigland wrote: Hi, We've been working on a program called sync_manager that implements shared-storage-based leases to protect shared resources. One way we'd like to use it is to protect vm images that reside on shared storage, i.e. preventing two vm's on two hosts from using the same image at once. There's two different, but related problems here: - Preventing 2 different VMs using the same disk - Preventing the same VM running on 2 hosts at once The first requires that there is a lease per configured disk (since a guest can have multiple disks). The latter requires a lease per VM and can ignore specifices of what disks are configured. IIUC, sync-manager is aiming for the latter. The present integration effort is aiming for the latter. sync_manager itself aims to be agnostic about what it's managing. Ok, it makes a bit of a difference to how we integrate with it in libvirt. If we want to ever let sync-manager do per-disk leases then we'd want to pass more info to the SM callbacks so it knows what disks QEMU has, instead of just its name It's functional, and the next big step is using it through libvirt. sync_manager wraps a process, i.e. acquires a lease, forksexecs a process, renews the lease wile the process runs, and releases the lease when the process exits. While the process runs, it has exclusive access to whatever resource was named in the lease that was acquired. There are complications around migration we need to consider too. During migration, you actually need QEMU running on two hosts at once. IIRC the idea is that before starting the migration operation, we'd have to tell sync-manager to mark the lease as shared with a specific host. The destination QEMU would have to startup in shared mode, and upgrade this to an exclusive lock when migration completes, or quit when migration fails. sync_manager leases can only be exclusive, so it's a matter of transfering ownership of the exclusive lock from source host to destination host. We have not yet added lease transfer capabilities to sync_manager, but it might look something like this: In the past I've discussed with the original SM authors the idea of introducing a shared lease concept. The idea was - QEMU is running on S with an exclusive lease - User requests migration to D - SM on S downgrades the exclusive lease to a shared lease, shared only with host D - libvirt starts QEMU on D host to accept migration - SM on D grabs the exclusive lease - libvirt starts migration on S - If migration succeeds - libvirt kills QEMU on S - SM on D upgrades its shared lease to exclusive - If migration fails - libvirt kills QEMU on D - SM on S upgrades its shared lease to exclusive S = source host, sm-S = sync_manager on S, ... D = destination host, sm-D = sync_manager on D, ... 1. sm-S holds the lease, and is monitoring qemu 2. migration begins from S to D 3. libvirt-D runs sm-D: sync_manager -c qemu with the addition of a new sync_manager option --receive-lease 4. sm-D writes its hostid D to the lease area signaling sm-S that it wants to be the lease owner when S is done with it 5. sm-D begins monitoring the lease owner on disk (which is still S) 6. sm-D forks qemu-D 7. sm-S sees that D wants the lease 8. qemu-S exits with success 9. sm-S sees qemu-S exit with success 10. sm-S writes D as the lease owner into the lease area and exits (in the non-migration/transfer case, sm-S writes owner=LEASE_FREE) 11. sm-D (still monitoring the lease owner) sees that it has become the owner, and begins renewing the lease 12. qemu-D runs fully I don't know enough (anything) about qemu migration yet to say if those steps work correctly or safely. One concern is that qemu-D should not enter a state where it can write until we are certain that D has been written as the lease's owner. Unfortunately the way migration works with QEMU prevents this scenario. This led us to invent the idea of a shared lease that is only used during migration. The one further complication is with the security drivers. IIUC, we will absolutely not want QEMU to have any access to the shared storage lease area. The problem is that if we just inject the wrapper process as is, sync-manager will end up running with exact same privileges as QEMU. ie same UID:GID, and same selinux context. I'm really not at all sure how to deal with this problem, because our core design is that the thing we spawn inherits the privileges we setup at fork() time. We don't want to delegate the security setup to sync-manager, because it introduces a huge variable condition in the security system. We need guarenteed consistent security setup for QEMU,
Re: [libvirt] using sync_manager with libvirt
On Wed, Aug 11, 2010 at 03:07:29PM -0600, Eric Blake wrote: On 08/11/2010 02:53 PM, Chris Lalancette wrote: Unfortunately, this is not how migration works in qemu/kvm. Using your nomenclature above, it's more like the following: A guest is running on S. A migration is then initiated, at which point D fires up a qemu process with a -incoming argument. This is sort of a container process that will receive all of the migration data. Crucially for sync-manager, though, qemu completely starts up and attaches to all of the resources (including disks) *while* qemu at S is still running. Then it enters a sort of paused state (where the guest cannot run), and receives all of the migration data. Once all of the migration data has been received, the guest on S is destroyed, and the guest on D is unpaused. That's why Dan mentioned that we need two hosts to access the disk at once. On the other hand, does D do any writes to the disk prior to the point at which it is unpaused? Would it work if D can be granted a read-only lease to access to the disk for the duration of the migration data, and then be converted over to read-write at the point when S is destroyed? Even if sync_manager had read/write lease semantics, this use case wouldn't translate onto it, because S is in write mode the entire time that D is in read mode, and read locks are not compatible with write locks. sync_manager shouldn't be viewed as something that's trying to add any new protection to the migration case. It's just trying to accurately represent, on disk, where qemu is unpaused. Dave -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] using sync_manager with libvirt
On Wed, Aug 11, 2010 at 05:19:27PM -0400, David Teigland wrote: On Wed, Aug 11, 2010 at 04:53:20PM -0400, Chris Lalancette wrote: 1. sm-S holds the lease, and is monitoring qemu 2. migration begins from S to D 3. libvirt-D runs sm-D: sync_manager -c qemu with the addition of a new sync_manager option --receive-lease 4. sm-D writes its hostid D to the lease area signaling sm-S that it wants to be the lease owner when S is done with it 5. sm-D begins monitoring the lease owner on disk (which is still S) 6. sm-D forks qemu-D 7. sm-S sees that D wants the lease 8. qemu-S exits with success 9. sm-S sees qemu-S exit with success 10. sm-S writes D as the lease owner into the lease area and exits (in the non-migration/transfer case, sm-S writes owner=LEASE_FREE) 11. sm-D (still monitoring the lease owner) sees that it has become the owner, and begins renewing the lease 12. qemu-D runs fully Unfortunately, this is not how migration works in qemu/kvm. Using your nomenclature above, it's more like the following: A guest is running on S. A migration is then initiated, at which point D fires up a qemu process with a -incoming argument. libvirt starts qemu -incoming on D, right? So with sync_manager, libvirt would start: sync_manager --receive_lease -c qemu -incoming Yes that is correct This is sort of a container process that will receive all of the migration data. Crucially for sync-manager, though, qemu completely starts up and attaches to all of the resources (including disks) *while* qemu at S is still running. Then it enters a sort of paused state (where the guest cannot run), and receives all of the migration data. That should all be fine. Once all of the migration data has been received, the guest on S is destroyed, ok, sm-S sees qemu-S exit at that point. and the guest on D is unpaused. The critical bit would be ensuring that sm-S has written owner=D into the lease area before qemu-D is unpaused. Hooking into the sequence at that point in time might be too difficult or ugly, I don't know. The main hard bit here is that QEMU gives us no indication that migration has completed. We 'detect' it by issuing a 'cont' command to unpause the CPUs - this command blocks until migration is done. Clearly this won't work for SM, but this isn't SM's problem. We need to fix this in QEMU so that we get an async notification of migration completion, so we can then tell SM to upgrade the lease, before we then issue 'cont' to start CPUs. That's why Dan mentioned that we need two hosts to access the disk at once. It would be easiest, of course, if the lease owner always represented where qemu was running, but that obviously won't work with migration. So we have to settle for the lease owner always representing where qemu is unpaused. I think my other mail is in fact describing the same thing as you are, I was just using different terminology :-) Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Explicitly disable unused floppy0 device for GSX
2010/8/11 Daniel Veillard veill...@redhat.com: On Mon, Aug 09, 2010 at 04:44:14PM -0600, Eric Blake wrote: On 08/05/2010 06:45 PM, Matthias Bolte wrote: floppy0.present defaults to true for GSX. Therefore, it needs to be explicitly disabled for GSX. --- src/esx/esx_vmx.c | 10 ++ 1 files changed, 10 insertions(+), 0 deletions(-) If I understand correctly, you want to uniformly make floppy not present except when the user asked for it. The other ESX clients meet this goal, but for GSX, you have to add an extra argument to make it so. That being the case, I agree with the approach. That's the intention, but it didn't struck me right away that ESX and GSX being different here is odd. I noticed that floppy0.present defaults to true on GSX while investigating some other GSX related issues in the VMX handling code. The comments in the esx_vmx.c file indicate that ESX would default to false in that case, but that's wrong, as I know now. floppy0.present defaults to true for GSX _and_ ESX, so the assumed default for ESX was wrong all the time. I probably screwed it when I wrote the initial VMX handling code. Therefore, this patch is incomplete and we need a v2 to address the ESX issue too. @@ -2539,6 +2544,11 @@ esxVMX_FormatConfig(esxVMX_Context *ctx, virCapsPtr caps, virDomainDefPtr def, } } + if (!hasFloppyDevice productVersion esxVI_ProductVersion_GSX) { I'd add an extra () here to make precedence clear: if (!hasFloppyDevice (productVersion esxVI_ProductVersion_GSX)) { ACK with that nit fixed. v2 won't have the product version check anymore. Agreed, ACK Matthias you push it ? Daniel Well, no, as we need a v2 for this :) Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] esx: Explicitly disable unused floppy devices
floppy0.present defaults to true. Therefore, it needs to be explicitly set to false when the XML config doesn't specify the corresponding floppy device. --- v2: - This issue affects ESX too, the assumed default was wrong in general - Explicitly disable unused floppy devices independent of the product version src/esx/esx_vmx.c | 19 +++ src/esx/esx_vmx.h |2 +- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/esx/esx_vmx.c b/src/esx/esx_vmx.c index d5d9ff0..12cd005 100644 --- a/src/esx/esx_vmx.c +++ b/src/esx/esx_vmx.c @@ -215,7 +215,7 @@ def-disks[0]... ## disks: floppy from .flp image ### -floppy0.present = true # defaults to false +floppy0.present = true # defaults to true floppy0.startConnected = true # defaults to true floppy0.clientDevice = false # defaults to false @@ -235,7 +235,7 @@ def-disks[0]... ## disks: floppy from host device ## -floppy0.present = true # defaults to false +floppy0.present = true # defaults to true floppy0.startConnected = true # defaults to true floppy0.clientDevice = false # defaults to false @@ -2320,6 +2320,7 @@ esxVMX_FormatConfig(esxVMX_Context *ctx, virCapsPtr caps, virDomainDefPtr def, virBuffer buffer = VIR_BUFFER_INITIALIZER; bool scsi_present[4] = { false, false, false, false }; int scsi_virtualDev[4] = { -1, -1, -1, -1 }; +bool floppy_present[2] = { false, false }; if (ctx-formatFileName == NULL) { ESX_ERROR(VIR_ERR_INTERNAL_ERROR, %s, @@ -2525,7 +2526,8 @@ esxVMX_FormatConfig(esxVMX_Context *ctx, virCapsPtr caps, virDomainDefPtr def, break; case VIR_DOMAIN_DISK_DEVICE_FLOPPY: -if (esxVMX_FormatFloppy(ctx, def-disks[i], buffer) 0) { +if (esxVMX_FormatFloppy(ctx, def-disks[i], buffer, +floppy_present) 0) { goto failure; } @@ -2539,6 +2541,13 @@ esxVMX_FormatConfig(esxVMX_Context *ctx, virCapsPtr caps, virDomainDefPtr def, } } +for (i = 0; i 2; ++i) { +/* floppy[0..1].present defaults to true, disable it explicitly */ +if (! floppy_present[i]) { +virBufferVSprintf(buffer, floppy%d.present = \false\\n, i); +} +} + /* def:fss */ /* FIXME */ @@ -2810,7 +2819,7 @@ esxVMX_FormatCDROM(esxVMX_Context *ctx, virDomainDiskDefPtr def, int esxVMX_FormatFloppy(esxVMX_Context *ctx, virDomainDiskDefPtr def, -virBufferPtr buffer) +virBufferPtr buffer, bool floppy_present[2]) { int unit; char *fileName = NULL; @@ -2824,6 +2833,8 @@ esxVMX_FormatFloppy(esxVMX_Context *ctx, virDomainDiskDefPtr def, return -1; } +floppy_present[unit] = true; + virBufferVSprintf(buffer, floppy%d.present = \true\\n, unit); if (def-type == VIR_DOMAIN_DISK_TYPE_FILE) { diff --git a/src/esx/esx_vmx.h b/src/esx/esx_vmx.h index a77264a..12fc5af 100644 --- a/src/esx/esx_vmx.h +++ b/src/esx/esx_vmx.h @@ -137,7 +137,7 @@ esxVMX_FormatCDROM(esxVMX_Context *ctx, virDomainDiskDefPtr def, int esxVMX_FormatFloppy(esxVMX_Context *ctx, virDomainDiskDefPtr def, -virBufferPtr buffer); +virBufferPtr buffer, bool floppy_present[2]); int esxVMX_FormatEthernet(virDomainNetDefPtr def, int controller, -- 1.7.0.4 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Improve VMX file name parsing and formatting
2010/8/11 Daniel Veillard veill...@redhat.com: On Sat, Aug 07, 2010 at 09:54:34PM +0200, Matthias Bolte wrote: 2010/8/7 Matthias Bolte matthias.bo...@googlemail.com: For parsing try to match by datastore mount path first, if that fails fallback to /vmfs/volumes/datastore/path parsing. This also fixes problems with GSX on Windows. Because GSX on Windows doesn't use /vmfs/volumes/ style file names. For formatting use the datastore mount path too, instead of using /vmfs/volumes/datastore/path as fixed format. --- src/esx/esx_driver.c | 372 +++-- 1 files changed, 235 insertions(+), 137 deletions(-) + + /* Strip trailing separators */ + length = strlen(hostMount-mountInfo-path); + + if (length 0) { + tmp = hostMount-mountInfo-path + length - 1; + + while (*tmp == separator tmp hostMount-mountInfo-path) { + --tmp; } [skip removed code] + length = tmp - hostMount-mountInfo-path; + } + + /* Format as mount[/directory]/file */ + virBufferAdd(buffer, hostMount-mountInfo-path, length); + The trailing separators stripping it totally broken :( This incremental diff fixes that: diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index 322c588..4fb357b 100644 --- a/src/esx/esx_driver.c +++ b/src/esx/esx_driver.c @@ -286,14 +286,8 @@ esxFormatVMXFileName(const char *datastorePath, void *opaque) /* Strip trailing separators */ length = strlen(hostMount-mountInfo-path); - if (length 0) { - tmp = hostMount-mountInfo-path + length - 1; - - while (*tmp == separator tmp hostMount-mountInfo-path) { - --tmp; - } - - length = tmp - hostMount-mountInfo-path; + while (length 0 hostMount-mountInfo-path[length - 1] == separator) { + --length; } /* Format as mount[/directory]/file */ Okay, ACK :-) Daniel Thanks, pushed. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Use MD5 sum of mount path as storage pool UUID
2010/8/9 Justin Clift jcl...@redhat.com: On 08/09/2010 06:56 AM, Matthias Bolte wrote: With the previous storage pool UUID source not all storage pools had a proper UUID, especially GSX storage pools. The mount path is unique per host and cannot change during the lifetime of the datastore. Therefore, its MD5 sum can be used as UUID. Use gnulib's crypto/md5 module to generate the MD5 sum. Just as a general thought, would it be preferably to use the SHA1 function instead of the MD5 one? Every non-trivial hash function will do for the purpose of deriving a UUID from the mount path. Asking because MD5 seems to be trending out of favour (as a generalisation) compared to SHA1, after some cryptographers showed MD5 hash collisions could be practically achieved with some types of data. (or something along those lines) I'm aware of MD5 being considered as broken for cryptographic use cases, but this one isn't cryptographic so I don't care about the problem of crafted collisions here. The only real problem here would be when you have two datastores with different mount paths that hash to the same UUID, but I think md5 is collision-free enough so we can ignore this problem. There is another problem with the approach of using the mount path hash as UUID. In case of a Windows based GSX server you can use CIFS shares as datastores addressed via UNC paths like \\nas\share. If you have multiple GSX server sharing that datastore then all will have the same UUID because the have the same mount path. Actually I don't consider this as a real problem because in this case there is one datastore with one UUID shared between multiple hosts. For all intents and purposes, I'm thinking it'll make no practical functional difference, but figure it's worth asking. :) gnulib also provides SHA1 and SHA256 so we could use those instead of MD5, but I think MD5 is good enough for this special use case here. Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Use MD5 sum of mount path as storage pool UUID
Anno domini 2010 Matthias Bolte scripsit: [...] There is another problem with the approach of using the mount path hash as UUID. In case of a Windows based GSX server you can use CIFS shares as datastores addressed via UNC paths like \\nas\share. If you have multiple GSX server sharing that datastore then all will have the same UUID because the have the same mount path. Actually I don't consider this as a real problem because in this case there is one datastore with one UUID shared between multiple hosts. Would this become a problem if the user would use \\nas one node and \\nas.dom.aim on another node? The nasty thing about network paths is that people might use aliases.. This just came to my mind. Ciao Max -- Gib Dein Bestes. Dann übertriff Dich selbst! -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] esx: Use MD5 sum of mount path as storage pool UUID
On Thu, Aug 12, 2010 at 01:40:14AM +0200, Matthias Bolte wrote: 2010/8/9 Justin Clift jcl...@redhat.com: On 08/09/2010 06:56 AM, Matthias Bolte wrote: With the previous storage pool UUID source not all storage pools had a proper UUID, especially GSX storage pools. The mount path is unique per host and cannot change during the lifetime of the datastore. Therefore, its MD5 sum can be used as UUID. Use gnulib's crypto/md5 module to generate the MD5 sum. Just as a general thought, would it be preferably to use the SHA1 function instead of the MD5 one? Every non-trivial hash function will do for the purpose of deriving a UUID from the mount path. Asking because MD5 seems to be trending out of favour (as a generalisation) compared to SHA1, after some cryptographers showed MD5 hash collisions could be practically achieved with some types of data. (or something along those lines) I'm aware of MD5 being considered as broken for cryptographic use cases, but this one isn't cryptographic so I don't care about the problem of crafted collisions here. The only real problem here would be when you have two datastores with different mount paths that hash to the same UUID, but I think md5 is collision-free enough so we can ignore this problem. There is another problem with the approach of using the mount path hash as UUID. In case of a Windows based GSX server you can use CIFS shares as datastores addressed via UNC paths like \\nas\share. If you have multiple GSX server sharing that datastore then all will have the same UUID because the have the same mount path. Actually I don't consider this as a real problem because in this case there is one datastore with one UUID shared between multiple hosts. This is actually a feature! If the same storage pool is visible on multiple hosts, it is absolutely desirable that it have the same UUID on each host. For all intents and purposes, I'm thinking it'll make no practical functional difference, but figure it's worth asking. :) gnulib also provides SHA1 and SHA256 so we could use those instead of MD5, but I think MD5 is good enough for this special use case here. Agreed, we don't need cryptographic security for this use case, so MD5 is fine. Daniel -- |: Red Hat, Engineering, London-o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org-o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list