Re: [libvirt] Question about compilation on MinGW(StorageAPI)
Hi, Dan
How about this?
With this patch,
make,
make check,
make syntax-check on libvirt is running (some test are skipped.).
At this moment "make install" is failed on virsh install like this.
The output is like thisat src directory
# ../libtool --mode=install /usr/bin/install -c 'virsh.exe'
'/usr/local/bin/virsh.exe'
../libtool: ./virsh.: No such file or directory
I am investigating this issue at this moment.
qemud/Makefile.am |2 ++
src/driver.h |4
src/internal.h|2 ++
src/libvirt.c |4
src/qemu_driver.c |8
src/remote_internal.c |6 ++
tests/Makefile.am |9 ++---
tests/testutils.c |5 -
tests/testutilsqemu.c |3 ++-
tests/virshtest.c |7 ++-
10 files changed, 44 insertions(+), 6 deletions(-)
Thanks
Atsushi SAKAI
"Daniel P. Berrange" <[EMAIL PROTECTED]> wrote:
> On Mon, Jun 09, 2008 at 12:05:09PM +0900, Atsushi SAKAI wrote:
> > Hi, Dan
> >
> > If remove uncompiled code on MinGW, the patch seems like this.
> >
> > 0)siginfo_t does not have.
> > 1)MinGW does not have waitpid, fork etc.
> > 2)add testutilsqemu.c to check WITH_QEMU
> >
> > I need to investigate more?
> > (Please wait a week, if need to investigate this.)
> >
> > src/driver.h |4
> > src/internal.h|4
> > src/libvirt.c |3 ++-
>
> I don't like this change - we should disable all the virStateXXX
> funtions, based on #ifdef WITH_LIBVIRTD, since they're only used
> when in daemon mode, and thus avoid a #ifndef WIN32
>
> > tests/testutils.c |9 -
>
> This change will cause the virshtest test file to fail every time on win32.
>
> The better way to approach this is to wrap the *entire* of
>
> virtTestCaptureProgramOutput
>
>
> in #ifndef WIN32, and then in the virshtest.c file disable the entire
> test suite,
>
>#ifndef WIN32
> normal test code...
>#else
>int main (void) { return (77); /* means 'test skipped' for automake */ }
>#endif
>
>
> See qemuxml2argvtest.c for an example of this.
>
> > tests/testutilsqemu.c |2 ++
>
> This is fine.
>
> Regards,
> Daniel.
> --
> |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :|
> |: http://libvirt.org -o- http://virt-manager.org -o- http://ovirt.org :|
> |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :|
> |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|
fix_compile_on_mingw.patch
Description: Binary data
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt passes wrong MAC address to KVM
Hi Daniel, thanks for reply. Well, virsh reports already WRONG address. I'll try other network types and some more debugging tomorrow and report further. cheers nik On Tue, Jun 17, 2008 at 12:09:03PM -0400, Daniel Veillard wrote: > Hum, I can't really reproduce this. I used a definition like: > > > > > > > > since i don't have bridging set. When starting the guest with 0.4.2 I see > > /usr/bin/qemu-kvm -M pc -m 512 -smp 1 -monitor pty -boot c -hda > /virt/test-mac.img -net nic,macaddr=00:16:3e:0d:24:00,vlan=0 -net > tap,fd=14,script=,vlan=0,ifname=vnet0 -usb -vnc 127.0.0.1:0 -k en-us > > when starting with 0.4.3 I see > > /usr/bin/qemu-kvm -S -M pc -m 512 -smp 1 -name test-mac -monitor pty -boot c > -drive file=/virt/test-mac.img,if=ide,index=0,boot=on -drive > file=,if=ide,media=cdrom,index=2 -net nic,macaddr=00:16:3e:0d:24:00,vlan=0 > -net tap,fd=13,script=,vlan=0,ifname=vnet0 -serial none -parallel none -usb > -vnc 127.0.0.1:0 -k en-us > > but in both cases the MAC parsing looks correct and properly provided down > to qemu-kvm . Could you double check with virsh directly (though i don't see > how python could scramble just that part of the XML description) using > virsh dumpxml and virsh create ... > Also testing with a network type of setting may help finding what is happening > precisely here > > 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/ > -- - Nikola CIPRICH LinuxBox.cz, s.r.o. 28. rijna 168, 709 01 Ostrava tel.: +420 596 603 142 fax:+420 596 621 273 mobil: +420 777 093 799 www.linuxbox.cz mobil servis: +420 737 238 656 email servis: [EMAIL PROTECTED] - -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] libvirt passes wrong MAC address to KVM
On Tue, Jun 17, 2008 at 04:18:59PM +0200, Nikola Ciprich wrote: > Hi all, > I think there could be a bug in libvirt passing MAC address to KVM. > If I have following in my xml config: > > > > > > and start the domain (I'm using python binding btw), qemu is then executed > with following parameter: > > ... -net nic,macaddr=00:16:3e:18:24:00,vlan=0 ... > > MAC address given in config is not used by any other machine. Does somebody > have a tip on where the problem could be? > I'm using libvirt-0.4.3. Hum, I can't really reproduce this. I used a definition like: since i don't have bridging set. When starting the guest with 0.4.2 I see /usr/bin/qemu-kvm -M pc -m 512 -smp 1 -monitor pty -boot c -hda /virt/test-mac.img -net nic,macaddr=00:16:3e:0d:24:00,vlan=0 -net tap,fd=14,script=,vlan=0,ifname=vnet0 -usb -vnc 127.0.0.1:0 -k en-us when starting with 0.4.3 I see /usr/bin/qemu-kvm -S -M pc -m 512 -smp 1 -name test-mac -monitor pty -boot c -drive file=/virt/test-mac.img,if=ide,index=0,boot=on -drive file=,if=ide,media=cdrom,index=2 -net nic,macaddr=00:16:3e:0d:24:00,vlan=0 -net tap,fd=13,script=,vlan=0,ifname=vnet0 -serial none -parallel none -usb -vnc 127.0.0.1:0 -k en-us but in both cases the MAC parsing looks correct and properly provided down to qemu-kvm . Could you double check with virsh directly (though i don't see how python could scramble just that part of the XML description) using virsh dumpxml and virsh create ... Also testing with a network type of setting may help finding what is happening precisely here 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
[libvirt] libvirt passes wrong MAC address to KVM
Hi all, I think there could be a bug in libvirt passing MAC address to KVM. If I have following in my xml config: and start the domain (I'm using python binding btw), qemu is then executed with following parameter: ... -net nic,macaddr=00:16:3e:18:24:00,vlan=0 ... MAC address given in config is not used by any other machine. Does somebody have a tip on where the problem could be? I'm using libvirt-0.4.3. Thanks in advance BR nik -- - Nikola CIPRICH LinuxBox.cz, s.r.o. 28. rijna 168, 709 01 Ostrava tel.: +420 596 603 142 fax:+420 596 621 273 mobil: +420 777 093 799 www.linuxbox.cz mobil servis: +420 737 238 656 email servis: [EMAIL PROTECTED] - -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5]: Rewrite findLuns function
Chris Lalancette wrote: > Reposting the 5/5 patch with Jim's minor comments fixed up. Compile tested > only > with his changes. > > This rather large patch rewrites the virStorageBackendISCSIFindLUNs() function > to only rely on sysfs for finding LUNs, given a session number. Along the > way, > it also fixes the bug where we wouldn't find LUNs for older kernels (with the > block:sda format), and also (possibly) fixes a race condition where we could > try > to find the LUN before udev has finished connecting it. I say it "possibly" > fixes it because I haven't been able to hit it so far, but I definitely need > more testing to try and confirm. > > Changes since last time: > 1) Don't blindly ignore the 0'th LUN (thanks Stefan). Instead, look if the > LUNs are Direct-Access devices (as defined by the kernel); if so, they are > LUNs > we can use. > 2) Fix up whitespace damage. > 3) Check the return value of the scsidev strdup as pointed out by Jim. > 4) Change all ISCSIADM commands to be const char *const as pointed out by > Jim. > > Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]> Committed Chris Lalancette -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 4/5]: Move the sendtarget command into the login command
Chris Lalancette wrote: > A small bugfix; we only need to call the iscsiadm sendtarget command when we > are > first logging in; we don't need to do it for logout. Move the sendtarget > command into the Login() function. > > Changes since last time: > 1) Make const char *cmdsendtarget into const char *const cmdsendtarget based > on > feedback from Jim. > > Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]> Committed Chris Lalancette -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 3/5]: Fix up the call to iscsiadm --mode session
Chris Lalancette wrote: > Older versions of iscsiadm didn't support the "-P 0" flag to the "iscsiadm > --mode session" command. However, just running "iscsiadm --mode session" > seems > to work on all version of iscsiadm commands back to FC-6, so just use that. > > Changes since last time: > None > > Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]> Committed Chris Lalancette -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/5]: Cleanup carriage returns in util.c
Chris Lalancette wrote: > In src/util.c, virLog is just a wrapper around fprintf(stderr). Make sure to > put line breaks at the end of lines that use virLog() (noticed during > testing). > > Changes since last time: > None > > Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]> Committed Chris Lalancette -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 1/5]: Return exitstatus from virStorageBackendRunProgRegex
Chris Lalancette wrote: > This patch changes things around so that virStorageBackendRunProgRegex() does > *not* virStorageReportError() if the fork()/exec() process it spawned > returned a > != 0 exit code. Rather, it returns the exitcode in this case, and it is up to > the higher level to determine whether this is a fatal error or not. The use > case for this change is in the iSCSI stuff; older versions of iscsiadm tools > would return a failure when getting the session number, despite the command > succeeding. > > Changes since last time: > 1) Fix up whitespace damage > > Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]> Committed Chris Lalancette -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5]: Rewrite findLuns function
On Tue, Jun 17, 2008 at 01:02:30PM +0200, Chris Lalancette wrote: > Reposting the 5/5 patch with Jim's minor comments fixed up. Compile tested > only > with his changes. > > This rather large patch rewrites the virStorageBackendISCSIFindLUNs() function > to only rely on sysfs for finding LUNs, given a session number. Along the > way, > it also fixes the bug where we wouldn't find LUNs for older kernels (with the > block:sda format), and also (possibly) fixes a race condition where we could > try > to find the LUN before udev has finished connecting it. I say it "possibly" > fixes it because I haven't been able to hit it so far, but I definitely need > more testing to try and confirm. I guess it got sufficient review and feedback, +1 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 4/5]: Move the sendtarget command into the login command
On Mon, Jun 16, 2008 at 03:52:46PM +0200, Chris Lalancette wrote: > A small bugfix; we only need to call the iscsiadm sendtarget command when we > are > first logging in; we don't need to do it for logout. Move the sendtarget > command into the Login() function. Again I don't know the details, but patch looks okay +1 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/5]: Fix up the call to iscsiadm --mode session
On Mon, Jun 16, 2008 at 03:52:42PM +0200, Chris Lalancette wrote: > Older versions of iscsiadm didn't support the "-P 0" flag to the "iscsiadm > --mode session" command. However, just running "iscsiadm --mode session" > seems > to work on all version of iscsiadm commands back to FC-6, so just use that. I'm fine with the but i can't appreciate the details :-) 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 2/5]: Cleanup carriage returns in util.c
On Mon, Jun 16, 2008 at 03:52:36PM +0200, Chris Lalancette wrote: > In src/util.c, virLog is just a wrapper around fprintf(stderr). Make sure to > put line breaks at the end of lines that use virLog() (noticed during > testing). Looks fine to me, but IMHO we should instead switch to the normal libvirt error reporting, +1 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 1/5]: Return exitstatus from virStorageBackendRunProgRegex
On Mon, Jun 16, 2008 at 03:52:31PM +0200, Chris Lalancette wrote: > This patch changes things around so that virStorageBackendRunProgRegex() does > *not* virStorageReportError() if the fork()/exec() process it spawned > returned a > != 0 exit code. Rather, it returns the exitcode in this case, and it is up to > the higher level to determine whether this is a fatal error or not. The use > case for this change is in the iSCSI stuff; older versions of iscsiadm tools > would return a failure when getting the session number, despite the command > succeeding. Looks fine to me, +1 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 0/5]: Rework iSCSI lun handling
On Mon, Jun 16, 2008 at 04:17:04PM +0200, Stefan de Konink wrote: > > Does RHEL-5 vintage have libvirt? yes, 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: Helper for enumerations
On Sun, Jun 15, 2008 at 09:59:38PM +0100, Daniel P. Berrange wrote: > There are quite a few places in our code where we have to convert from > a string to an int enumeration, and vica-verca. This is tedious code to > write, and I'm about to introduce a tonne more enumerations in the new > generic domain XML parser / generator. So I reckon its time for some > helper APIs Looks fine to me, +1 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 5/5]: Rewrite findLuns function
Reposting the 5/5 patch with Jim's minor comments fixed up. Compile tested only
with his changes.
This rather large patch rewrites the virStorageBackendISCSIFindLUNs() function
to only rely on sysfs for finding LUNs, given a session number. Along the way,
it also fixes the bug where we wouldn't find LUNs for older kernels (with the
block:sda format), and also (possibly) fixes a race condition where we could try
to find the LUN before udev has finished connecting it. I say it "possibly"
fixes it because I haven't been able to hit it so far, but I definitely need
more testing to try and confirm.
Changes since last time:
1) Don't blindly ignore the 0'th LUN (thanks Stefan). Instead, look if the
LUNs are Direct-Access devices (as defined by the kernel); if so, they are LUNs
we can use.
2) Fix up whitespace damage.
3) Check the return value of the scsidev strdup as pointed out by Jim.
4) Change all ISCSIADM commands to be const char *const as pointed out by Jim.
Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]>
diff -urp libvirt.1to4/src/storage_backend_iscsi.c libvirt.5/src/storage_backend_iscsi.c
--- libvirt.1to4/src/storage_backend_iscsi.c 2008-06-17 12:52:44.0 +0200
+++ libvirt.5/src/storage_backend_iscsi.c 2008-06-17 12:54:21.0 +0200
@@ -119,7 +119,7 @@ virStorageBackendISCSISession(virConnect
int vars[] = {
2,
};
-const char *prog[] = {
+const char *const prog[] = {
ISCSIADM, "--mode", "session", NULL
};
char *session = NULL;
@@ -153,7 +153,7 @@ virStorageBackendISCSIConnection(virConn
const char *portal,
const char *action)
{
-const char *cmdargv[] = {
+const char *const cmdargv[] = {
ISCSIADM, "--mode", "node", "--portal", portal,
"--targetname", pool->def->source.devices[0].path, action, NULL
};
@@ -164,110 +164,30 @@ virStorageBackendISCSIConnection(virConn
return 0;
}
-
static int
-virStorageBackendISCSIMakeLUN(virConnectPtr conn,
- virStoragePoolObjPtr pool,
- char **const groups,
- void *data)
+virStorageBackendISCSINewLun(virConnectPtr conn, virStoragePoolObjPtr pool,
+ unsigned int lun, const char *dev)
{
virStorageVolDefPtr vol;
int fd = -1;
-unsigned int target, channel, id, lun;
-char lunid[100];
-int opentries = 0;
char *devpath = NULL;
-char *session = data;
-char sysfs_path[PATH_MAX];
-char *dev = NULL;
-DIR *sysdir;
-struct dirent *block_dirent;
-struct stat sbuf;
-int len;
-
-if ((virStrToLong_ui(groups[0], NULL, 10, &target) < 0) ||
-(virStrToLong_ui(groups[1], NULL, 10, &channel) < 0) ||
-(virStrToLong_ui(groups[2], NULL, 10, &id) < 0) ||
-(virStrToLong_ui(groups[3], NULL, 10, &lun) < 0)) {
-virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
- _("Failed parsing iscsiadm commands"));
-return -1;
-}
-
-if (lun == 0) {
-/* the 0'th LUN isn't a real LUN, it's just a control LUN; skip it */
-return 0;
-}
-
-snprintf(sysfs_path, PATH_MAX,
- "/sys/class/iscsi_session/session%s/device/"
- "target%d:%d:%d/%d:%d:%d:%d/block",
- session, target, channel, id, target, channel, id, lun);
-
-if (stat(sysfs_path, &sbuf) < 0) {
-/* block path in subdir didn't exist; this is unexpected, so fail */
-virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _("Failed to find the sysfs path for %d:%d:%d:%d: %s"),
- target, channel, id, lun, strerror(errno));
-return -1;
-}
-
-sysdir = opendir(sysfs_path);
-if (sysdir == NULL) {
-/* we failed for some reason; return an error */
-virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
- _("Failed to opendir sysfs path %s: %s"),
- sysfs_path, strerror(errno));
-return -1;
-}
-
-while ((block_dirent = readdir(sysdir)) != NULL) {
-len = strlen(block_dirent->d_name);
-if ((len == 1 && block_dirent->d_name[0] == '.') ||
-(len == 2 && block_dirent->d_name[0] == '.' && block_dirent->d_name[1] == '.')) {
-/* the . and .. directories; just skip them */
-continue;
-}
-
-/* OK, not . or ..; let's see if it is a SCSI device */
-if (len > 2 &&
-block_dirent->d_name[0] == 's' &&
-block_dirent->d_name[1] == 'd') {
-/* looks like a scsi device, smells like scsi device; it must be
- a scsi device */
-dev = strdup(block_dirent->d_name);
-break;
-}
-}
-closedir(sysdir);
-
-if (dev == NULL) {
-/* we didn't find the sd? device we were
Re: [libvirt] [PATCH 5/5]: Rewrite findLuns function
Jim Meyering wrote:
>> diff -urp libvirt.sendtarget/src/storage_backend_iscsi.c
>> libvirt.findLun/src/storage_backend_iscsi.c
>> --- libvirt.sendtarget/src/storage_backend_iscsi.c 2008-06-16
>> 14:35:34.0 +0200
>> +++ libvirt.findLun/src/storage_backend_iscsi.c 2008-06-16
>> 14:52:31.0 +0200
>> @@ -119,7 +119,7 @@ virStorageBackendISCSISession(virConnect
> ...
>> +while ((sys_dirent = readdir(sysdir))) {
>> +/* double-negative, so we can use the same function for scandir
>> below */
>> +if (!notdotdir(sys_dirent))
>> +continue;
>> +
>> +if (STREQLEN(sys_dirent->d_name, "target", 6)) {
>> +if (sscanf(sys_dirent->d_name, "target%u:%u:%u",
>> + &host, &bus, &target) != 3) {
>> +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
>> + _("Failed to parse target in sysfs
>> path %s: %s"),
>> + sysfs_path, strerror(errno));
>
> You can remove the ": %s" suffix and the strerror(errno) argument,
> since errno isn't relevant here. Or maybe better: leave the suffix
> and use sys_dirent->d_name as the argument, so the diagnostic shows
> what couldn't be parsed.
Oops! Cut and pasted that from elsewhere, and didn't fix up the error messages.
I'll fix that up as you advise.
>
>> +closedir(sysdir);
>> +return -1;
>> +
>
> The above line has just three TABs.
> Best to delete it.
Yeah, probably leftover from my whitespace fixup. I'll fix that as well.
>
> The rest, including patches 1-4, looks fine,
> so, pending whatever tests you deem adequate,
> ACK.
Thanks for the review!
Chris Lalancette
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 5/5]: Rewrite findLuns function
Chris Lalancette <[EMAIL PROTECTED]> wrote:
> This rather large patch rewrites the virStorageBackendISCSIFindLUNs() function
> to only rely on sysfs for finding LUNs, given a session number. Along the
> way,
> it also fixes the bug where we wouldn't find LUNs for older kernels (with the
> block:sda format), and also (possibly) fixes a race condition where we could
> try
> to find the LUN before udev has finished connecting it. I say it "possibly"
> fixes it because I haven't been able to hit it so far, but I definitely need
> more testing to try and confirm.
>
> Changes since last time:
> 1) Don't blindly ignore the 0'th LUN (thanks Stefan). Instead, look if the
> LUNs are Direct-Access devices (as defined by the kernel); if so, they are
> LUNs
> we can use.
> 2) Fix up whitespace damage.
> 3) Check the return value of the scsidev strdup as pointed out by Jim.
> 4) Change all ISCSIADM commands to be const char *const as pointed out by
> Jim.
>
> diff -urp libvirt.sendtarget/src/storage_backend_iscsi.c
> libvirt.findLun/src/storage_backend_iscsi.c
> --- libvirt.sendtarget/src/storage_backend_iscsi.c2008-06-16
> 14:35:34.0 +0200
> +++ libvirt.findLun/src/storage_backend_iscsi.c 2008-06-16
> 14:52:31.0 +0200
> @@ -119,7 +119,7 @@ virStorageBackendISCSISession(virConnect
...
> +while ((sys_dirent = readdir(sysdir))) {
> +/* double-negative, so we can use the same function for scandir
> below */
> +if (!notdotdir(sys_dirent))
> +continue;
> +
> +if (STREQLEN(sys_dirent->d_name, "target", 6)) {
> +if (sscanf(sys_dirent->d_name, "target%u:%u:%u",
> + &host, &bus, &target) != 3) {
> +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
> + _("Failed to parse target in sysfs
> path %s: %s"),
> + sysfs_path, strerror(errno));
You can remove the ": %s" suffix and the strerror(errno) argument,
since errno isn't relevant here. Or maybe better: leave the suffix
and use sys_dirent->d_name as the argument, so the diagnostic shows
what couldn't be parsed.
> +closedir(sysdir);
> +return -1;
> +
The above line has just three TABs.
Best to delete it.
The rest, including patches 1-4, looks fine,
so, pending whatever tests you deem adequate,
ACK.
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH]: fix ruby-libvirt bindings when virConnectNum* returns 0
On Tue, Jun 17, 2008 at 11:16:31AM +0200, Chris Lalancette wrote: > Attached is a simple patch to fix a problem I ran into when using the > ruby-libvirt bindings with libvirt 0.4.3. Basically, if you call any of the > virConnectList* functions with a "max" of 0, it returns "Invalid Arg". To get > around this, modify the ruby-libvirt bindings to return an empty list if we > get > num == 0 when calling the corresponding virConnectNumOf* function. This > should > solve: https://bugzilla.redhat.com/show_bug.cgi?id=451666 Hum, looking at the bugzilla it's for storage that the problem was raised. It's a bit annoying, the main domain and network functions accept a max = 0 The documentation of virConnectListDefinedStoragePools doesn't prevent maxnames = 0 Actually the check done in the function is if ((names == NULL) || (maxnames < 0)) i.e. it allows 0, and pass it to the underlying driver. One solution would be to just return 0 there, as attached in this patch, the other solution would be to check which underlying storage driver failed and fix it, 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/libvirt.c === RCS file: /data/cvs/libxen/src/libvirt.c,v retrieving revision 1.146 diff -u -p -r1.146 libvirt.c --- src/libvirt.c 10 Jun 2008 10:43:28 - 1.146 +++ src/libvirt.c 17 Jun 2008 09:59:00 - @@ -4023,6 +4023,9 @@ virConnectListStoragePools(virConnectPt return (-1); } +if (maxnames == 0) +return (0); + if (conn->storageDriver && conn->storageDriver->listPools) return conn->storageDriver->listPools (conn, names, maxnames); @@ -4087,6 +4090,9 @@ virConnectListDefinedStoragePools(virCon return (-1); } +if (maxnames == 0) +return(0); + if (conn->storageDriver && conn->storageDriver->listDefinedPools) return conn->storageDriver->listDefinedPools (conn, names, maxnames); -- Libvir-list mailing list Libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH]: fix ruby-libvirt bindings when virConnectNum* returns 0
On Tue, Jun 17, 2008 at 11:16:31AM +0200, Chris Lalancette wrote:
> Attached is a simple patch to fix a problem I ran into when using the
> ruby-libvirt bindings with libvirt 0.4.3. Basically, if you call any of the
> virConnectList* functions with a "max" of 0, it returns "Invalid Arg". To get
> around this, modify the ruby-libvirt bindings to return an empty list if we
> get
> num == 0 when calling the corresponding virConnectNumOf* function. This
> should
> solve: https://bugzilla.redhat.com/show_bug.cgi?id=451666
That doesn't sound proper to me. I re-checked, all virConnectList* functions
in libvirt.c raise with an error only if the connection is invalid, if
the pointer to the storage is NULL or if the max value is < 0. the max value
being 0 should be accepted, that's the semantic of the check done on top
of the driver. So what driver did fail with an argument error ? this need
to be fixed.
This looks okay in the QEmu driver from what I saw
xenHypervisorListDomains seems to need fixing.
xenDaemonListDomains need to be fixed too.
xenDaemonListDefinedDomains need to be fixed too.
The OpenVZ driver looks okay
The LXC driver looks okay
The remote driver seems okay but it's harder for me to assert it works
through the full chain.
Patch for the Xen parts attached.
Could you try to investigate and find more precisely under what conditions
you got the 'max = 0' returning an invalid arg error, we need to find this
fix it not work around it at the binding level IMHO,
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/xen_internal.c
===
RCS file: /data/cvs/libxen/src/xen_internal.c,v
retrieving revision 1.121
diff -u -p -r1.121 xen_internal.c
--- src/xen_internal.c 6 Jun 2008 11:09:57 - 1.121
+++ src/xen_internal.c 17 Jun 2008 09:48:17 -
@@ -2554,9 +2554,12 @@ xenHypervisorListDomains(virConnectPtr c
priv = (xenUnifiedPrivatePtr) conn->privateData;
if (priv->handle < 0 ||
-(ids == NULL) || (maxids < 1))
+(ids == NULL) || (maxids < 0))
return (-1);
+if (maxids == 0)
+return(0);
+
if (!(XEN_GETDOMAININFOLIST_ALLOC(dominfos, maxids))) {
virXenError(conn, VIR_ERR_NO_MEMORY, "allocating %d domain info",
maxids);
Index: src/xend_internal.c
===
RCS file: /data/cvs/libxen/src/xend_internal.c,v
retrieving revision 1.198
diff -u -p -r1.198 xend_internal.c
--- src/xend_internal.c 9 Jun 2008 12:16:03 - 1.198
+++ src/xend_internal.c 17 Jun 2008 09:48:17 -
@@ -3330,7 +3330,10 @@ xenDaemonListDomains(virConnectPtr conn,
struct sexpr *_for_i, *node;
long id;
-if ((ids == NULL) || (maxids <= 0))
+if (maxids == 0)
+return(0);
+
+if ((ids == NULL) || (maxids < 0))
goto error;
root = sexpr_get(conn, "/xend/domain");
if (root == NULL)
@@ -4219,8 +4222,11 @@ int xenDaemonListDefinedDomains(virConne
if (priv->xendConfigVersion < 3)
return(-1);
-if ((names == NULL) || (maxnames <= 0))
+if ((names == NULL) || (maxnames < 0))
goto error;
+if (maxnames == 0)
+return(0);
+
root = sexpr_get(conn, "/xend/domain?state=halted");
if (root == NULL)
goto error;
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH]: fix ruby-libvirt bindings when virConnectNum* returns 0
Attached is a simple patch to fix a problem I ran into when using the
ruby-libvirt bindings with libvirt 0.4.3. Basically, if you call any of the
virConnectList* functions with a "max" of 0, it returns "Invalid Arg". To get
around this, modify the ruby-libvirt bindings to return an empty list if we get
num == 0 when calling the corresponding virConnectNumOf* function. This should
solve: https://bugzilla.redhat.com/show_bug.cgi?id=451666
Signed-off-by: Chris Lalancette <[EMAIL PROTECTED]>
diff -r b02171cfad7d ext/libvirt/_libvirt.c
--- a/ext/libvirt/_libvirt.c Wed Apr 16 09:47:27 2008 -0700
+++ b/ext/libvirt/_libvirt.c Tue Jun 17 11:06:26 2008 +0200
@@ -256,7 +256,11 @@ static VALUE vol_new(virStorageVolPtr n,
\
num = virConnectNumOf##objs(conn); \
_E(num < 0, create_error(e_RetrieveError, "virConnectNumOf" # objs, "", conn)); \
-\
+if (num == 0) { \
+/* if num is 0, don't call virConnectList* function */ \
+result = rb_ary_new2(num); \
+return result; \
+} \
names = ALLOC_N(char *, num); \
r = virConnectList##objs(conn, names, num); \
if (r < 0) {\
--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
