Re: [Libvir] [PATCH] reindent __virErrorMsg with spaces instead of tabs
On Sun, Mar 23, 2008 at 07:48:58PM +0100, Guido Günther wrote: __virErrorMsg is a mix of tabs and spaces which makes it a bit hard to read. This patch cleans this up. Please apply. Hum, could you send those kind of patches as attachments in the future ? Basically assuming spaces/tabs are correctly preserved in an email body is taking unecessary risks. Also having a name for the patch and being able to download them from on-line email archives is a good thing, Now for the content of the patch, it seems to remove all tabs and replace them with 8 spaces. Is that really a better way, why use 8 characters when you can use one for the same purpose ? One could argue either way, maybe we should standardize on tabs, maybe we should replace them all, but I'm not sure the second one is really the right way. At the moment nearly all C files uses tabs, and i know I'm using them to align parameters lists in headers too, so starting a tab replacement would hit nearly all the code. Also make sure you editor use tab to align to the next 8 character boundary. To me that renders correctly in vim (I have tabstop=8). 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: [Libvir] [PATCH] Add Xen Scheduler APIs to xenDaemonDriver
On Mon, Mar 24, 2008 at 02:12:25PM +0900, Saori Fukuta wrote: Hello, Thanks for applying the first patch (about adding read-only connections). http://git.et.redhat.com/?p=libvirt.git;a=commit;h=4faeb0d567976377b2661fb5ebd0f1dca7763f80 How about the second one ? We can make some improvements for virsh schedinfo command with the patch, so I want the patch to be committed if it is OK. I had forgotten about it, sorry. It looks okay, though i have to clean a few problems raised by 'make syntax-check' (please try this in future patches). I commited it to CVS. However, i don't see why it is needed, basically when running with Xen as root, the hypervisor access should be used, not xend, so this seems to indicate that you have problem using the hypervisor calls and that's probably something to worry about. But it's fine having a more complete xend driver surrport. 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: [Libvir] Patch for routed virtual networks
Anything further I can do to help get this patch commited? I have been running with it, without problems across restarts, etc., for a couple of weeks now. man, 10 03 2008 kl. 22:09 +0100, skrev Mads Chr. Olesen: søn, 09 03 2008 kl. 21:09 +, skrev Daniel P. Berrange: On Sat, Mar 08, 2008 at 04:33:32PM +0100, Mads Chr. Olesen wrote: I have added a route dev=ethX / stanza (dev is optional), completely equivalent to the forward / stanza. This is still forwarding of traffic, so I think we should just use the existing forward/ element and have an extra attribute to indiciate the type of forwarding, eg forward/ (defaults to mode=nat for compat) forward mode=nat/ forward mode=route/ forward mode=nat dev=ethX/ forward mode=route dev=ethX/ Sure, makes sense - an updated patch is attached. I'm a little unclear on how this actually works. You add iptables rules to allow traffic in/out, but you're not adding any routing table entries, nor turning on proxy_arp, so I don't see how this will actually work in practice. Are you assuming the admin has already added suitable routing rules turned on proxy arp ? Well, in my case (dedicated server, hetzner.de) this is all that is needed. My physical interface has IP 85.10.XXX.XXX, and then I have a secondary IP range which gets routed at that interface, IP range 78.47.YYY.YYY/30. I then setup my virtual interface with an IP in that range, by setting ip address=78.47.YYY.YYY netmask=255.255.255.248 / Thus, to get packets routed at the virtual machines, it just needs to be allowed by iptables, and /proc/sys/net/ipv4/ip_forward needs to be set to 1. Other setups obviously might need more work. -- Mads Chr. Olesen [EMAIL PROTECTED] shiyee.dk -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [RFC] 3 of 4 Linux Container support
On Fri, Mar 21, 2008 at 02:14:38PM -0700, Dave Leskovec wrote: Daniel P. Berrange wrote: The following patches make the driver properly integrate with the stateful driver APIs. It also changes the config files to be named by on VM name instead of UUID, since this is what the QEMU driver does its more user friendly. It also adds the CLONE_XXX constants since they have not yet been added to the libc sched.h file. IMHO we should enable the driver by default, since it can already probe for availability at runtime. Finally it also fixes a typo where it wrote 'linuxcontainer' as the domain type in the config file instead of 'lxc'. Thanks, this looks great. Back when I posted the first pass of these patches, you mentioned it should be defaulted to enabled only on Linux. I'm not an autoconf expert (or intermediate for that matter). How do we default the enabled parm based on the host os? Hum, maybe in configure.in if test $with_lxc = yes ; then could be tweaked to use the `uname` = Linux condition too. But I don't know if the AC_ARG_WITH(lxc, [ --with-lxc add Linux Container support (off)],[],[with_lxc=no]) can be enclosed in a similar if test, probably worth trying 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: [Libvir] [PATCH] Add Xen Scheduler APIs to xenDaemonDriver
On Mon, 24 Mar 2008 05:27:04 -0400 Daniel Veillard wrote: On Mon, Mar 24, 2008 at 02:12:25PM +0900, Saori Fukuta wrote: How about the second one ? We can make some improvements for virsh schedinfo command with the patch, so I want the patch to be committed if it is OK. I had forgotten about it, sorry. It looks okay, though i have to clean a few problems raised by 'make syntax-check' (please try this in future patches). I commited it to CVS. Thank you for applying it. I will try syntax-check next time. However, i don't see why it is needed, basically when running with Xen as root, the hypervisor access should be used, not xend, so this seems to indicate that you have problem using the hypervisor calls and that's probably something to worry about. But it's fine having a more complete xend driver surrport. Yes, the hypervisor access should be used for root user. But if a domain is inactive, we cannot see/set the scheduler parameters with visrh command even though xm command can do it since we cannot access to the hypervisor. And also, we cannot allow to see the scheduler parameters for user who connect with read-only for the same reason. For such case, I thought we needed the patch to access with Xend driver. thanks a lot, Saori. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] mark error messages as translatable
Daniel Veillard [EMAIL PROTECTED] wrote: On Sun, Mar 23, 2008 at 07:50:24PM +0100, Guido Günther wrote: This patch marks the error messages in qemu_driver.c as translatable, some of them were marked as such already. Please apply. [...] qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - cannot create bridge '%s' : %s, name, strerror(err)); + _(cannot create bridge '%s' : %)), name, strerror(err)); Patch is a bit broken here, otherwise that looks fine. I'm wondering why that wasn't check by 'make syntax-check' as other modules are checked for translatable strings. I explained why, fixed the underlying problem, and fixed about 1/3 of the violations it exposed with this big patch: http://thread.gmane.org/gmane.comp.emulators.libvirt/5374 But for your objection, I would have applied that change weeks ago. I've been waiting for a reply to my question at the end of the thread: http://thread.gmane.org/gmane.comp.emulators.libvirt/5374/focus=5382 -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [Libvir] [PATCH] Add Xen Scheduler APIs to xenDaemonDriver
On Mon, Mar 24, 2008 at 07:37:51PM +0900, Saori Fukuta wrote: Thank you for applying it. I will try syntax-check next time. thanks :-) However, i don't see why it is needed, basically when running with Xen as root, the hypervisor access should be used, not xend, so this seems to indicate that you have problem using the hypervisor calls and that's probably something to worry about. But it's fine having a more complete xend driver surrport. Yes, the hypervisor access should be used for root user. But if a domain is inactive, we cannot see/set the scheduler parameters hum, right i forgot the inactive domain problem. with visrh command even though xm command can do it since we cannot access to the hypervisor. And also, we cannot allow to see the scheduler parameters for user who connect with read-only for the same reason. Well read-only should still connect to the daemon, at least to read the status, the proxy mode should be deprecated, except for older setups. For such case, I thought we needed the patch to access with Xend driver. Okay, understood. But we have been bitten with the hypervisor being broken with xen-3.2 and not realizing it because things were done (in a far more expensive way) via xend. 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: [Libvir] [PATCH] mark error messages as translatable
On Mon, Mar 24, 2008 at 12:26:51PM +0100, Jim Meyering wrote: Daniel Veillard [EMAIL PROTECTED] wrote: On Sun, Mar 23, 2008 at 07:50:24PM +0100, Guido Günther wrote: This patch marks the error messages in qemu_driver.c as translatable, some of them were marked as such already. Please apply. [...] qemudReportError(conn, NULL, NULL, VIR_ERR_INTERNAL_ERROR, - cannot create bridge '%s' : %s, name, strerror(err)); + _(cannot create bridge '%s' : %)), name, strerror(err)); Patch is a bit broken here, otherwise that looks fine. I'm wondering why that wasn't check by 'make syntax-check' as other modules are checked for translatable strings. I explained why, fixed the underlying problem, and fixed about 1/3 of the violations it exposed with this big patch: http://thread.gmane.org/gmane.comp.emulators.libvirt/5374 But for your objection, I would have applied that change weeks ago. I've been waiting for a reply to my question at the end of the thread: http://thread.gmane.org/gmane.comp.emulators.libvirt/5374/focus=5382 My recollection is that I objected to a bunch of non-translatable strings from the protocol module, Guido patch was about qemu_driver.c errors which had translatable meaning. Maybe there is some overlap, but it looks different. To me adding comments to C files to tag the fact that untranslatable strings are now marked as translatable to allow an automatic check in the hope that the translators will see those comments (with whatever tools they have) and won't loose time on those strings didn't made much sense to me (and still don't honnestly). Either remove the check from the protocol module and and commit the other parts, or just ignore me and commit the full thing. As you pointed out you're not responsible for the multiplication of those debugging strings in that module. 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
[Libvir] PATCH: Add SCSI HBA backend for storage driver
This patch adds a new backend to the storage driver supporting SCSI HBAs. With this you can now enumerate physical disks provided by FibreChannel, internal SCSI / PATA / SATA / USB adapters. The implemention uses HAL to enumerate the volumes asssociated with an HBA. It is common for most boxes to support multiple HBAs. eg on my Linux laptop $ ls /sys/class/scsi_host/ host0 host1 host2 host3 These HBA names are used to identify the host when defining the XML: pool type=scsi namehba0/name source adapter name=host0/ /source target path/dev/disk/by-path/path /target /pool This does not provide any means of discovering HBA names - that's a later piece of broader work on host device discovery. It also does not support multipath devices (eg with FibreChannel the same storage volume can appear on the host multiple times) Example using the first host adapter which is my internal SATA controller with a single disk attached: # ./src/virsh pool-create /root/hba0.xml Pool hba0 created from /root/hba0.xml # ./src/virsh vol-list hba0 Name Path - 0:0:0:0 /dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0 # ./src/virsh vol-info /dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0 Name: 0:0:0:0 Type: block Capacity: 153.39 GB Allocation: 153.39 GB # ./src/virsh vol-dumpxml /dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0 libvir: Storage error : no storage vol with matching key volume name0:0:0:0/name keySATA_HDT722516DLA380_VDK91GTE0WPLER/key source /source capacity16469620/capacity allocation16469620/allocation target path/dev/disk/by-path/pci-:00:1f.2-scsi-0:0:0:0/path permissions mode060640/mode owner0/owner group6/group labelsystem_u:object_r:fixed_disk_device_t/label /permissions /target /volume This also does not deal with the problem of HBAs being created on the fly, eg the NPIV case where you can add/remove adapters at will. I'm still working on how to deal with this. If you have your virtual NPIV adapters already defined though, it can enumerate them fine. Aside from the bit where I map from the short HBA name (eg 'host0') into the HAL device name, this should be portable to Solaris as-is. Dan. -- |: Red Hat, Engineering, Boston -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: [Libvir] PATCH: Add SCSI HBA backend for storage driver
And this time with the patch included... configure.in | 28 ++ src/Makefile.am| 10 src/storage_backend.c | 14 + src/storage_backend_scsi.c | 466 + src/storage_backend_scsi.h | 45 src/storage_conf.c |9 src/util.c |2 7 files changed, 571 insertions(+), 3 deletions(-) Dan. Index: configure.in === RCS file: /data/cvs/libvirt/configure.in,v retrieving revision 1.136 diff -u -p -r1.136 configure.in --- configure.in21 Mar 2008 15:03:37 - 1.136 +++ configure.in24 Mar 2008 23:05:25 - @@ -28,6 +28,7 @@ GNUTLS_REQUIRED=1.0.25 AVAHI_REQUIRED=0.6.0 POLKIT_REQUIRED=0.6 PARTED_REQUIRED=1.8.0 +HAL_REQUIRED=0.5.0 dnl Checks for C compiler. AC_PROG_CC @@ -584,6 +585,8 @@ AC_ARG_WITH(storage-iscsi, [ --with-storage-iscsiwith iSCSI backend for the storage driver (on)],[],[with_storage_iscsi=check]) AC_ARG_WITH(storage-disk, [ --with-storage-disk with GPartd Disk backend for the storage driver (on)],[],[with_storage_disk=check]) +AC_ARG_WITH(storage-scsi, +[ --with-storage-scsi with HAL SCSI Disk backend for the storage driver (on)],[],[with_storage_scsi=check]) if test $with_storage_fs = yes -o $with_storage_fs = check; then AC_PATH_PROG(MOUNT, [mount], [], [$PATH:/sbin:/usr/sbin]) @@ -741,6 +744,30 @@ AC_SUBST(LIBPARTED_CFLAGS) AC_SUBST(LIBPARTED_LIBS) +HAL_CFLAGS= +HAL_LIBS= +if test x$with_storage_scsi = xyes -o x$with_storage_scsi = xcheck; then + PKG_CHECK_MODULES(HAL, hal = $HAL_REQUIRED, +[with_storage_scsi=yes], [ +if test x$with_storage_scsi = xcheck ; then + with_storage_scsi=no +else + AC_MSG_ERROR( + [You must install HAL = $HAL_REQUIRED to compile libvirt]) +fi + ]) + if test x$with_storage_scsi = xyes ; then +AC_DEFINE_UNQUOTED(WITH_STORAGE_SCSI_, 1, + [use SCSI backend for storage driver is enabled]) + fi +fi +AC_DEFINE_UNQUOTED(WITH_STORAGE_SCSI, 1, [whether SCSI backend for storage driver is enabled]) +AM_CONDITIONAL(WITH_STORAGE_SCSI, [test yes = yes]) +AC_SUBST(HAL_CFLAGS) +AC_SUBST(HAL_LIBS) + + + dnl dnl check for python dnl @@ -968,6 +995,7 @@ AC_MSG_NOTICE([ NetFS: $with_storage_f AC_MSG_NOTICE([ LVM: $with_storage_lvm]) AC_MSG_NOTICE([ iSCSI: $with_storage_iscsi]) AC_MSG_NOTICE([Disk: $with_storage_disk]) +AC_MSG_NOTICE([SCSI: $with_storage_scsi]) AC_MSG_NOTICE([]) AC_MSG_NOTICE([Libraries]) AC_MSG_NOTICE([]) Index: src/Makefile.am === RCS file: /data/cvs/libvirt/src/Makefile.am,v retrieving revision 1.75 diff -u -p -r1.75 Makefile.am --- src/Makefile.am 21 Mar 2008 15:03:37 - 1.75 +++ src/Makefile.am 24 Mar 2008 23:05:25 - @@ -9,6 +9,7 @@ INCLUDES = \ $(GNUTLS_CFLAGS) \ $(SASL_CFLAGS) \ $(SELINUX_CFLAGS) \ + $(HAL_CFLAGS) \ -DBINDIR=\$(libexecdir)\ \ -DSBINDIR=\$(sbindir)\ \ -DSYSCONF_DIR=\$(sysconfdir)\ \ @@ -89,11 +90,18 @@ else EXTRA_DIST += storage_backend_disk.h storage_backend_disk.c endif +if WITH_STORAGE_SCSI +CLIENT_SOURCES += storage_backend_scsi.h storage_backend_scsi.c +else +EXTRA_DIST += storage_backend_scsi.h storage_backend_scsi.c +endif + libvirt_la_SOURCES = $(CLIENT_SOURCES) $(SERVER_SOURCES) -libvirt_la_LIBADD = $(LIBXML_LIBS) $(GNUTLS_LIBS) $(SASL_LIBS) $(SELINUX_LIBS) \ +libvirt_la_LIBADD = $(LIBXML_LIBS) $(GNUTLS_LIBS) $(SASL_LIBS) \ +$(SELINUX_LIBS) $(HAL_LIBS) \ @CYGWIN_EXTRA_LIBADD@ ../gnulib/lib/libgnu.la libvirt_la_LDFLAGS = -Wl,--version-script=$(srcdir)/libvirt_sym.version \ -version-info @LIBVIRT_VERSION_INFO@ \ Index: src/storage_backend.c === RCS file: /data/cvs/libvirt/src/storage_backend.c,v retrieving revision 1.10 diff -u -p -r1.10 storage_backend.c --- src/storage_backend.c 17 Mar 2008 16:57:21 - 1.10 +++ src/storage_backend.c 24 Mar 2008 23:05:25 - @@ -45,6 +45,9 @@ #if WITH_STORAGE_DISK #include storage_backend_disk.h #endif +#if WITH_STORAGE_SCSI +#include storage_backend_scsi.h +#endif #include util.h @@ -67,6 +70,9 @@ static virStorageBackendPtr backends[] = #if WITH_STORAGE_DISK virStorageBackendDisk, #endif +#if WITH_STORAGE_SCSI +virStorageBackendSCSI, +#endif }; @@ -121,6 +127,10 @@ virStorageBackendFromString(const char * if (STREQ(type, disk)) return VIR_STORAGE_POOL_DISK; #endif +#if WITH_STORAGE_SCSI +if (STREQ(type, scsi)) +return VIR_STORAGE_POOL_SCSI; +#endif virStorageReportError(NULL, VIR_ERR_INTERNAL_ERROR, _(unknown storage backend type %s), type); @@ -150,6 +160,10 @@
Re: [Libvir] [PATCH] Another Report error in virsh.c code.
Hi, Would you give me a comment on my opinion ? If not, I make a patch that is suitable for my opinion. Shigeki Sakamoto. $ virsh vcpupin error: command 'vcpupin' requires domain option error: command 'vcpupin' requires vcpu option error: command 'vcpupin' requires cpulist option These messages that vshCommandCheckOpts outputs is no problem. What I meant to say was that I want to apply following rules of migrate to vcpupin. ***migrate*** - when option desturi error It outputs error message in vshCommandCheckOpts. error: command 'migrate' requires desturi option - when internal-error occurs at vshCommandOptString It outputs error message after vshCommandOptString. migrate: Missing desturi ***vcpupin*** - when option cpulist error It outputs error message in vshCommandCheckOpts. error: command 'vcpupin' requires cpulist option - when internal-error occurs at vshCommandOptString ***no error-message, and return 1*** -I want to output error-message here, because I want to tell an error to a user. Regards, Shigeki Sakamoto. -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
