[libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Chris Lalancette
To support LVM partitioning in oVirt, one of the things we need is the ability
to tell what kind of label is currently on a block device.  Here, a 'label' is
used in the same sense that it is used in parted; namely, it defines which kind
of partition table is on the disk, whether it be DOS, LVM2, SUN, BSD, etc.  Note
that this is different than the partition type; those are things like Linux,
FAT16, FAT32, etc.

This actually turns out to be fairly easy to implement; there are really only a
few labels that are in common use, and they all have an easy signature to
recognize (but see comments in the code about pc98).  This patch implements
label detection on block devices in virStorageBackendUpdateVolInfoFD, and hooks
it up to the iSCSI backend so it works there.

To keep code duplication down, I moved some of the enum's from
storage_backend_disk.c into a common place.  Note, however, that there is a
slight semantic change because of this.  Previously, if no label was found on a
disk in storage_backend_disk.c, it would always return dos as the label type.
 That's not actually true, though; if it's a completely zeroed disk, for
instance, it really just has label type of 'unknown'.  This patch changes to the
new semantic of 'unknown' for label types we don't understand.  I don't think
this will be a huge issue for compatibility, but there could be something I'm
missing.

Otherwise, this patch has been tested by me to work, and now when you do:

# virsh vol-dumpxml --pool iscsitest lun-1

you'll get:

volume
...
  target
  ...
  format type='dos' /

Which should be sufficient for oVirt to do it's detection.

Signed-off-by: Chris Lalancette [EMAIL PROTECTED]
Index: src/storage_backend.c
===
RCS file: /data/cvs/libvirt/src/storage_backend.c,v
retrieving revision 1.21
diff -u -r1.21 storage_backend.c
--- src/storage_backend.c	5 Sep 2008 12:03:45 -	1.21
+++ src/storage_backend.c	16 Oct 2008 07:29:46 -
@@ -192,6 +192,30 @@
 return ret;
 }
 
+struct diskType disk_types[] = {
+{ lvm2, VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CUL },
+{ gpt,  VIR_STORAGE_POOL_DISK_GPT,  0x200, 8, 0x5452415020494645UL },
+{ dvh,  VIR_STORAGE_POOL_DISK_DVH,  0x0,   4, 0x41A9E50BUL },
+{ mac,  VIR_STORAGE_POOL_DISK_MAC,  0x0,   2, 0x5245UL },
+{ bsd,  VIR_STORAGE_POOL_DISK_BSD,  0x40,  4, 0x82564557UL },
+{ sun,  VIR_STORAGE_POOL_DISK_SUN,  0x1fc, 2, 0xBEDAUL },
+/*
+ * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), so
+ * we can't use that.  At the moment I'm relying on the dummy IPL
+ * bootloader data that comes from parted.  Luckily, the chances of running
+ * into a pc98 machine running libvirt are approximately nil.
+ */
+/*{ pc98, 0x1fe, 2, 0xAA55UL },*/
+{ pc98, VIR_STORAGE_POOL_DISK_PC98, 0x0,   8, 0x314C504900CBUL },
+/*
+ * NOTE: the order is important here; some other disk types (like GPT and
+ * and PC98) also have 0x55AA at this offset.  For that reason, the DOS
+ * one must be the last one.
+ */
+{ dos,  VIR_STORAGE_POOL_DISK_DOS,  0x1fe, 2, 0xAA55UL },
+{ NULL,   -1, 0x0,   0, 0x0UL },
+};
+
 int
 virStorageBackendUpdateVolInfoFD(virConnectPtr conn,
  virStorageVolDefPtr vol,
@@ -245,6 +269,40 @@
 if (withCapacity) vol-capacity = end;
 }
 
+/* make sure to set the target format unknown to begin with */
+vol-target.format = VIR_STORAGE_POOL_DISK_UNKNOWN;
+
+if (S_ISBLK(sb.st_mode)) {
+off_t start;
+int i;
+unsigned char buffer[1024];
+ssize_t bytes;
+
+start = lseek(fd, 0, SEEK_SET);
+if (start  0) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+  _(cannot seek to beginning of file '%s':%s),
+  vol-target.path, strerror(errno));
+return -1;
+}
+memset(buffer, 0, 1024);
+bytes = saferead(fd, buffer, 1024);
+if (bytes  0) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+  _(cannot read beginning of file '%s':%s),
+  vol-target.path, strerror(errno));
+return -1;
+}
+
+for (i = 0; disk_types[i].name != NULL; i++) {
+if (memcmp(buffer+disk_types[i].offset, disk_types[i].magic,
+disk_types[i].length) == 0) {
+vol-target.format = disk_types[i].label;
+break;
+}
+}
+}
+
 vol-target.perms.mode = sb.st_mode;
 vol-target.perms.uid = sb.st_uid;
 vol-target.perms.gid = sb.st_gid;
@@ -348,6 +406,34 @@
 return devpath;
 }
 
+int
+virStorageBackendDiskLabelFormatFromString(virConnectPtr conn ATTRIBUTE_UNUSED,
+   const char *format) {
+  

[libvirt] (no subject)

2008-10-16 Thread Guido Günther
On Wed, Oct 15, 2008 at 09:59:12PM +0100, Daniel P. Berrange wrote:
 When you get to that level of cleverness, it seems to me that it is verging 
 on a complete re-implementation of DLM (distributed lock manager), which 
 really, AFAIK, needs a proper cluster setup so it can safely fence 
 mis-behaving nodes, and avoid quorum/split-brain problems.
I've been toying with the idea of using DLM for libvirt earlier this
year [1](but infered from other postings on the list that this would be out
of scope for libvirt - probably should have asked). I looked at vm based
locks then but having storage based locks is even better.

Currently you have to make sure manually that people using i.e.
virt-manager[2] don't accidentally fire up VMs managed via e.g.
rgmanager.

Having cluster wide storage based locks would be an awesome solution.
 -- Guido


[1] using the rather simple lock_resource() and unlock_resource() API:
  
http://git.fedorahosted.org/git/cluster.git?p=cluster.git;a=blob;f=dlm/doc/libdlm.txt
[2] i.e. by having virt-manager hooked to all libvirtds in the cluster
and allowing to start each uuid only once

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Jim Meyering
Chris Lalancette [EMAIL PROTECTED] wrote:
 To support LVM partitioning in oVirt, one of the things we need is the ability
 to tell what kind of label is currently on a block device.  Here, a 'label' is
 used in the same sense that it is used in parted; namely, it defines which 
 kind
 of partition table is on the disk, whether it be DOS, LVM2, SUN, BSD, etc.  
 Note
 that this is different than the partition type; those are things like Linux,
 FAT16, FAT32, etc.

 This actually turns out to be fairly easy to implement; there are really only 
 a
 few labels that are in common use, and they all have an easy signature to
 recognize (but see comments in the code about pc98).  This patch implements
 label detection on block devices in virStorageBackendUpdateVolInfoFD, and 
 hooks
 it up to the iSCSI backend so it works there.

 To keep code duplication down, I moved some of the enum's from
 storage_backend_disk.c into a common place.  Note, however, that there is a
 slight semantic change because of this.  Previously, if no label was found on 
 a
 disk in storage_backend_disk.c, it would always return dos as the label 
 type.
  That's not actually true, though; if it's a completely zeroed disk, for
 instance, it really just has label type of 'unknown'.  This patch changes to 
 the
 new semantic of 'unknown' for label types we don't understand.  I don't think
 this will be a huge issue for compatibility, but there could be something I'm
 missing.

Hi Chris,

I like the patch.
Some minor suggestions below.

 Otherwise, this patch has been tested by me to work, and now when you do:

 # virsh vol-dumpxml --pool iscsitest lun-1

 you'll get:

 volume
 ...
   target
   ...
   format type='dos' /

 Which should be sufficient for oVirt to do it's detection.

 Signed-off-by: Chris Lalancette [EMAIL PROTECTED]
 Index: src/storage_backend.c
 ===
 RCS file: /data/cvs/libvirt/src/storage_backend.c,v
 retrieving revision 1.21
 diff -u -r1.21 storage_backend.c
 --- src/storage_backend.c 5 Sep 2008 12:03:45 -   1.21
 +++ src/storage_backend.c 16 Oct 2008 07:29:46 -
 @@ -192,6 +192,30 @@
  return ret;
  }

 +struct diskType disk_types[] = {
 +{ lvm2, VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CUL },
 +{ gpt,  VIR_STORAGE_POOL_DISK_GPT,  0x200, 8, 0x5452415020494645UL },
 +{ dvh,  VIR_STORAGE_POOL_DISK_DVH,  0x0,   4, 0x41A9E50BUL },
 +{ mac,  VIR_STORAGE_POOL_DISK_MAC,  0x0,   2, 0x5245UL },
 +{ bsd,  VIR_STORAGE_POOL_DISK_BSD,  0x40,  4, 0x82564557UL },
 +{ sun,  VIR_STORAGE_POOL_DISK_SUN,  0x1fc, 2, 0xBEDAUL },

Some partition tables are deliberately created to be dual GPT/DOS-based.
Here, it gets the first match (as you've deliberately ordered them that
way, I see).  We might need a mechanism to let the caller prefer one or
the other.  Also, realize that this can be wrong if there was once a GPT
table, and it has since been replaced with a DOS one that still happens
to have the GPT signature bytes.  Or maybe it was dual GPT/DOS long ago
but has since been maintained only as DOS (in which case all GPT-related
headers could be gone).  This suggests that pronouncing a partition
table to have type GPT based solely on the first 1KB is a little risky.
But it's probably just fine for now.

 +/*
 + * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), 
 so
 + * we can't use that.  At the moment I'm relying on the dummy IPL
 + * bootloader data that comes from parted.  Luckily, the chances of 
 running
 + * into a pc98 machine running libvirt are approximately nil.

I agree.
Why not just exclude it?

 + */
 +/*{ pc98, 0x1fe, 2, 0xAA55UL },*/
 +{ pc98, VIR_STORAGE_POOL_DISK_PC98, 0x0,   8, 0x314C504900CBUL },
 +/*
 + * NOTE: the order is important here; some other disk types (like GPT and
 + * and PC98) also have 0x55AA at this offset.  For that reason, the DOS
 + * one must be the last one.
 + */
 +{ dos,  VIR_STORAGE_POOL_DISK_DOS,  0x1fe, 2, 0xAA55UL },
 +{ NULL,   -1, 0x0,   0, 0x0UL },
 +};
 +
  int
  virStorageBackendUpdateVolInfoFD(virConnectPtr conn,
   virStorageVolDefPtr vol,
 @@ -245,6 +269,40 @@
  if (withCapacity) vol-capacity = end;
  }

 +/* make sure to set the target format unknown to begin with */
 +vol-target.format = VIR_STORAGE_POOL_DISK_UNKNOWN;
 +
 +if (S_ISBLK(sb.st_mode)) {
 +off_t start;
 +int i;
 +unsigned char buffer[1024];
 +ssize_t bytes;
 +
 +start = lseek(fd, 0, SEEK_SET);
 +if (start  0) {
 +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
 +  _(cannot seek to beginning of file 
 '%s':%s),
 +  vol-target.path, strerror(errno));
 +return -1;
 +}
 +memset(buffer, 

Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Chris Lalancette
Chris Lalancette wrote:
 To support LVM partitioning in oVirt, one of the things we need is the ability
 to tell what kind of label is currently on a block device.  Here, a 'label' is
 used in the same sense that it is used in parted; namely, it defines which 
 kind
 of partition table is on the disk, whether it be DOS, LVM2, SUN, BSD, etc.  
 Note
 that this is different than the partition type; those are things like Linux,
 FAT16, FAT32, etc.

Updated patch based on jim's feedback.  I did not remove the pc98 type; if
everyone else thinks it's a good idea, I'll remove it, but it is part of the ABI
in some sense.  Otherwise, I followed all of jim's suggestions.

-- 
Chris Lalancette
Index: src/storage_backend.c
===
RCS file: /data/cvs/libvirt/src/storage_backend.c,v
retrieving revision 1.21
diff -u -r1.21 storage_backend.c
--- src/storage_backend.c	5 Sep 2008 12:03:45 -	1.21
+++ src/storage_backend.c	16 Oct 2008 09:49:01 -
@@ -192,6 +192,30 @@
 return ret;
 }
 
+struct diskType disk_types[] = {
+{ lvm2, VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CUL },
+{ gpt,  VIR_STORAGE_POOL_DISK_GPT,  0x200, 8, 0x5452415020494645UL },
+{ dvh,  VIR_STORAGE_POOL_DISK_DVH,  0x0,   4, 0x41A9E50BUL },
+{ mac,  VIR_STORAGE_POOL_DISK_MAC,  0x0,   2, 0x5245UL },
+{ bsd,  VIR_STORAGE_POOL_DISK_BSD,  0x40,  4, 0x82564557UL },
+{ sun,  VIR_STORAGE_POOL_DISK_SUN,  0x1fc, 2, 0xBEDAUL },
+/*
+ * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), so
+ * we can't use that.  At the moment I'm relying on the dummy IPL
+ * bootloader data that comes from parted.  Luckily, the chances of running
+ * into a pc98 machine running libvirt are approximately nil.
+ */
+/*{ pc98, 0x1fe, 2, 0xAA55UL },*/
+{ pc98, VIR_STORAGE_POOL_DISK_PC98, 0x0,   8, 0x314C504900CBUL },
+/*
+ * NOTE: the order is important here; some other disk types (like GPT and
+ * and PC98) also have 0x55AA at this offset.  For that reason, the DOS
+ * one must be the last one.
+ */
+{ dos,  VIR_STORAGE_POOL_DISK_DOS,  0x1fe, 2, 0xAA55UL },
+{ NULL,   -1, 0x0,   0, 0x0UL },
+};
+
 int
 virStorageBackendUpdateVolInfoFD(virConnectPtr conn,
  virStorageVolDefPtr vol,
@@ -245,6 +269,41 @@
 if (withCapacity) vol-capacity = end;
 }
 
+/* make sure to set the target format unknown to begin with */
+vol-target.format = VIR_STORAGE_POOL_DISK_UNKNOWN;
+
+if (S_ISBLK(sb.st_mode)) {
+off_t start;
+int i;
+unsigned char buffer[1024];
+ssize_t bytes;
+
+start = lseek(fd, 0, SEEK_SET);
+if (start  0) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+  _(cannot seek to beginning of file '%s':%s),
+  vol-target.path, strerror(errno));
+return -1;
+}
+bytes = saferead(fd, buffer, sizeof(buffer));
+if (bytes  0) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+  _(cannot read beginning of file '%s':%s),
+  vol-target.path, strerror(errno));
+return -1;
+}
+
+for (i = 0; disk_types[i].name != NULL; i++) {
+if (disk_types[i].offset + disk_types[i].length  bytes)
+continue;
+if (memcmp(buffer+disk_types[i].offset, disk_types[i].magic,
+disk_types[i].length) == 0) {
+vol-target.format = disk_types[i].part_table_type;
+break;
+}
+}
+}
+
 vol-target.perms.mode = sb.st_mode;
 vol-target.perms.uid = sb.st_uid;
 vol-target.perms.gid = sb.st_gid;
@@ -348,6 +407,34 @@
 return devpath;
 }
 
+int
+virStorageBackendDiskLabelFormatFromString(virConnectPtr conn ATTRIBUTE_UNUSED,
+   const char *format) {
+int i;
+
+if (format == NULL)
+return VIR_STORAGE_POOL_DISK_UNKNOWN;
+
+for (i = 0; disk_types[i].name != NULL; i++) {
+if (STREQ(format, disk_types[i].name))
+	return disk_types[i].part_table_type;
+}
+
+return VIR_STORAGE_POOL_DISK_UNKNOWN;
+}
+
+const char *
+virStorageBackendDiskLabelFormatToString(virConnectPtr conn ATTRIBUTE_UNUSED,
+ int format) {
+int i;
+
+for (i = 0; disk_types[i].name != NULL; i++) {
+if (format == disk_types[i].part_table_type)
+return disk_types[i].name;
+}
+
+return unknown;
+}
 
 #ifndef __MINGW32__
 /*
Index: src/storage_backend.h
===
RCS file: /data/cvs/libvirt/src/storage_backend.h,v
retrieving revision 1.7
diff -u -r1.7 storage_backend.h
--- src/storage_backend.h	2 Sep 2008 14:15:42 -	

Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Daniel P. Berrange
On Thu, Oct 16, 2008 at 09:43:58AM +0200, Chris Lalancette wrote:
 To keep code duplication down, I moved some of the enum's from
 storage_backend_disk.c into a common place.  Note, however, that there is a
 slight semantic change because of this.  Previously, if no label was found on 
 a
 disk in storage_backend_disk.c, it would always return dos as the label 
 type.
  That's not actually true, though; if it's a completely zeroed disk, for
 instance, it really just has label type of 'unknown'.  This patch changes to 
 the
 new semantic of 'unknown' for label types we don't understand.  I don't think
 this will be a huge issue for compatibility, but there could be something I'm
 missing.

I don't see a compatability problem - existing behaviour is a clear bug.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Chris Lalancette
Daniel P. Berrange wrote:
 
 I don't understand why it has to be 1-billion ? For the enum support to
 work correctly, we need this to be contiguous, starting from zero, and
 and as the last element have
 
  VIR_STORAGE_POOL_DISK_LAST
 
 This perhaps suggests that DISK_UNKNOWN should be the first entry so
 that if you have a zero'd struct with a disk type, it'll get defaulted
 to UNKOWN

Yeah, after changing the DOS type to 1, I now realize that UNKNOWN should be the
0'th entry.  I'll change that, change to the VIR_ENUM implementation as you
suggest, fix up the fallout, and repost.

Thanks,
-- 
Chris Lalancette

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Jim Meyering
Daniel P. Berrange [EMAIL PROTECTED] wrote:
 +struct diskType disk_types[] = {

 This can be const const - surprised Jim didn't catch this :-)

 +struct diskType disk_types[] = {

Had to leave some for you. :-P

Now that you mention it, though, that should be static, too.

 +enum partTableType {
 +VIR_STORAGE_POOL_DISK_DOS = 1,
 +VIR_STORAGE_POOL_DISK_DVH,
 +VIR_STORAGE_POOL_DISK_GPT,
 +VIR_STORAGE_POOL_DISK_MAC,
 +VIR_STORAGE_POOL_DISK_BSD,
 +VIR_STORAGE_POOL_DISK_PC98,
 +VIR_STORAGE_POOL_DISK_SUN,
 +VIR_STORAGE_POOL_DISK_LVM2,
 +/* the unknown disk is 1 billion (and not, for instance, -1), to make
 +   sure it doesn't run afoul of error checking */
 +VIR_STORAGE_POOL_DISK_UNKNOWN = 1 * 1024 * 1024 * 1024,

 I don't understand why it has to be 1-billion ? For the enum support to
 work correctly, we need this to be contiguous, starting from zero, and
 and as the last element have

  VIR_STORAGE_POOL_DISK_LAST

 This perhaps suggests that DISK_UNKNOWN should be the first entry so
 that if you have a zero'd struct with a disk type, it'll get defaulted
 to UNKOWN

Or maybe leave it as = 1, to force explicit definition to an enum value.
Then, wherever it's zeroed but not explicitly defined
it will show up as undefined (possibly triggering some sort
of error), rather than proceeding with a mere UNKNOWN.

I guess it depends on the relative worth of being able to
initialize with calloc-style allocation vs. cost of explicit
definition everywhere.

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Libvirt support for migration in KVM

2008-10-16 Thread Shanmuga Rajan
Hi

I have tested migration function VirDomain object by migrating with in
localhost(for testing purpose).

System details are

KVM hypervisor.
Intel VT 64 bit centos.
libvirt 0.4.6. (compiled from source)
vs i am running is Win 2003 64 bit...
Host is also 64 bit

but when i tried to use the migrate function of virDomain object. it
throws the following error

libvirt.libvirtError: virDomainMigrate() failed this function is not
supported by the hypervisor: virDomainMigrate


i read in kvm wiki that they do support  migration
Can anyone point out the problem


here comes the python code

 vsobj.migrate(conn,libvirt.VIR_MIGRATE_LIVE,None,conn.getURI(),0)
libvir: error : this function is not supported by the hypervisor:
virDomainMigrate
Traceback (most recent call last):
  File stdin, line 1, in ?
  File /usr/lib64/python2.4/site-packages/libvirt.py, line 301, in migrate
if ret is None:raise libvirtError('virDomainMigrate() failed', dom=self)
libvirt.libvirtError: virDomainMigrate() failed this function is not
supported by the hypervisor: virDomainMigrate




Thanks for any suggestion

-- Shan

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Chris Lalancette
Jim Meyering wrote:
 diff -u -r1.21 storage_backend.c
 --- src/storage_backend.c5 Sep 2008 12:03:45 -   1.21
 +++ src/storage_backend.c16 Oct 2008 07:29:46 -
 @@ -192,6 +192,30 @@
  return ret;
  }

 +struct diskType disk_types[] = {
 +{ lvm2, VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CUL },
 +{ gpt,  VIR_STORAGE_POOL_DISK_GPT,  0x200, 8, 0x5452415020494645UL },
 +{ dvh,  VIR_STORAGE_POOL_DISK_DVH,  0x0,   4, 0x41A9E50BUL },
 +{ mac,  VIR_STORAGE_POOL_DISK_MAC,  0x0,   2, 0x5245UL },
 +{ bsd,  VIR_STORAGE_POOL_DISK_BSD,  0x40,  4, 0x82564557UL },
 +{ sun,  VIR_STORAGE_POOL_DISK_SUN,  0x1fc, 2, 0xBEDAUL },
 
 Some partition tables are deliberately created to be dual GPT/DOS-based.
 Here, it gets the first match (as you've deliberately ordered them that
 way, I see).  We might need a mechanism to let the caller prefer one or
 the other.  Also, realize that this can be wrong if there was once a GPT
 table, and it has since been replaced with a DOS one that still happens
 to have the GPT signature bytes.  Or maybe it was dual GPT/DOS long ago
 but has since been maintained only as DOS (in which case all GPT-related
 headers could be gone).  This suggests that pronouncing a partition
 table to have type GPT based solely on the first 1KB is a little risky.
 But it's probably just fine for now.

Sigh, yes.  As you can probably tell, this code is heavily based off of the
implementation in parted.  I really just looked at the probe functions for
each of the label types (more on the label name below), and followed what parted
did.  In the GPT case, it seems like it just probes the front, but you've
pointed out via IRC that other routines do checking (like in the last sector,
for the backup signature).  Like you say, though, I *think* this will be OK for
now; if we really need it, we can add multiple signature checking later.

 
 +/*
 + * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), 
 so
 + * we can't use that.  At the moment I'm relying on the dummy IPL
 + * bootloader data that comes from parted.  Luckily, the chances of 
 running
 + * into a pc98 machine running libvirt are approximately nil.
 
 I agree.
 Why not just exclude it?

I'd be happy to.  I mostly included it because it was already there in the
storage_backend_disk implementation, and it was relatively easy to add.  If no
one else has objections, I'm happy to dump it over the side.

 +if (S_ISBLK(sb.st_mode)) {
 +off_t start;
 +int i;
 +unsigned char buffer[1024];
 +ssize_t bytes;
 +
 +start = lseek(fd, 0, SEEK_SET);
 +if (start  0) {
 +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
 +  _(cannot seek to beginning of file 
 '%s':%s),
 +  vol-target.path, strerror(errno));
 +return -1;
 +}
 +memset(buffer, 0, 1024);
 
 You can remove the above memset, given one of the suggestions below.
 
 +bytes = saferead(fd, buffer, 1024);
 
 Better not to repeat constants like that.  use sizeof:
 
bytes = saferead(fd, buffer, sizeof buffer);

Yeah, good point.  Will fix.

 
 +if (bytes  0) {
 +virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
 +  _(cannot read beginning of file 
 '%s':%s),
 +  vol-target.path, strerror(errno));
 +return -1;
 +}
 
 At worst, memset just the probably-empty portion of the buffer
 that wasn't set by the read:
 
memset(buffer + bytes, 0, sizeof buffer - bytes);
 
 However, I prefer to drop the memset altogether and check
 offset+len against bytes below:
 
 +for (i = 0; disk_types[i].name != NULL; i++) {
 
 It'd be good to check that offset+len is within range, regardless
 of whether the buffer is zeroed.  In case someone decides to shrink
 buffer or to add a type with magic bytes past the 1KB mark.
 
if (disk_types[i].offset + disk_types[i].length  bytes)
continue;

Ah, yes, I see.  Yeah, I thought about adding additional checking for this, but
just didn't do it.  Your suggestion is good, I'll do that.

 diff -u -r1.7 storage_backend.h
 --- src/storage_backend.h2 Sep 2008 14:15:42 -   1.7
 +++ src/storage_backend.h16 Oct 2008 07:29:46 -
 @@ -56,6 +56,30 @@
  VIR_STORAGE_BACKEND_POOL_SOURCE_NAME= (14),
  };

 +enum diskLabel {
 +VIR_STORAGE_POOL_DISK_DOS = 0,
 
 I like to start with 1 or larger, so that using an uninitialized
 variable (often 0) is more likely to trigger an error.

In general, I agree with you, and if I was writing this part from scratch I
would do that.  I left it as-is because this is the way it was already done.
Actually this goes along with the whole slightly changing semantics thing I
mentioned for storage_backend_disk.  Because the vol pointer is 

[libvirt] FYI: committed make syntax-check fix

2008-10-16 Thread Chris Lalancette
I just committed this change:

Wed Oct 16 14:03:00 CEST 2008 Chris Lalancette [EMAIL PROTECTED]

* make syntax-check was complaining that network_driver.c was
missing from POTFILES.in.  Add it, and then fix up one warning about
included c-ctypes.h that wasn't being used.

Since make syntax-check was complaining about the lack of network_driver.c.
Yes, I know today is Thursday, Oct. 16, I just noticed my silliness.  Anyway,
just wanted to give a heads up.

-- 
Chris Lalancette

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] Libvirt support for migration in KVM

2008-10-16 Thread Atsushi SAKAI
Hi,

Please read following message.

https://www.redhat.com/archives/libvir-list/2008-September/msg00011.html

Thanks
Atsushi SAKAI







Shanmuga Rajan [EMAIL PROTECTED] wrote:

 Hi
 
 I have tested migration function VirDomain object by migrating with in
 localhost(for testing purpose).
 
 System details are
 
 KVM hypervisor.
 Intel VT 64 bit centos.
 libvirt 0.4.6. (compiled from source)
 vs i am running is Win 2003 64 bit...
 Host is also 64 bit
 
 but when i tried to use the migrate function of virDomain object. it
 throws the following error
 
 libvirt.libvirtError: virDomainMigrate() failed this function is not
 supported by the hypervisor: virDomainMigrate
 
 
 i read in kvm wiki that they do support  migration
 Can anyone point out the problem
 
 
 here comes the python code
 
  vsobj.migrate(conn,libvirt.VIR_MIGRATE_LIVE,None,conn.getURI(),0)
 libvir: error : this function is not supported by the hypervisor:
 virDomainMigrate
 Traceback (most recent call last):
   File stdin, line 1, in ?
   File /usr/lib64/python2.4/site-packages/libvirt.py, line 301, in migrate
 if ret is None:raise libvirtError('virDomainMigrate() failed', dom=self)
 libvirt.libvirtError: virDomainMigrate() failed this function is not
 supported by the hypervisor: virDomainMigrate
 
 
 
 
 Thanks for any suggestion
 
 -- Shan
 
 --
 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


Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Daniel P. Berrange
On Thu, Oct 16, 2008 at 02:35:21PM +0200, Chris Lalancette wrote:
 Daniel P. Berrange wrote:
  I know you're just replicating the existing code, but both these functions 
  can
  be killed off and replaced with auto-generated code from our enum support
  macros
  
  VIR_ENUM_IMPL(virStorageBackendDiskLabel, 
VIR_STORAGE_POOL_DISK_LAST,
dos, dvh, gpt, mac,
bsd, pc98, sun, lvm2
unknown)
  
  And in the header file just
  
  VIR_ENUM_DECL(virStorageBackendDiskLabel)
 
 I've implemented this, and there was quite a bit of fallout from it.  In
 particular, I had to change the signature of poolOptions.formatToString and
 formatFromString, which required a bunch of changes elsewhere in the storage
 code.  The good news is that we remove quite a bit of very similar hand 
 crafted
 code, so this patch actually removes more code than it adds.  Tested by me as
 before.
 diff -u -r1.15 storage_backend_fs.c
 --- a/src/storage_backend_fs.c13 Oct 2008 16:46:29 -  1.15
 +++ b/src/storage_backend_fs.c16 Oct 2008 12:31:23 -
 @@ -48,7 +48,8 @@
  #include xml.h
  
  enum {
 -VIR_STORAGE_POOL_FS_AUTO = 0,
 +VIR_STORAGE_POOL_FS_UNKNOWN = 0,
 +VIR_STORAGE_POOL_FS_AUTO = 1,

This shouldn't be added - automatic is intended to be default.

  
  enum {
 -VIR_STORAGE_VOL_RAW,
 +VIR_STORAGE_VOL_UNKNOWN = 0,
 +VIR_STORAGE_VOL_RAW = 1,

Likewise, 'raw' should be the default, with no need for unknown.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] fix make syntax-check vs. new .ico file with trailing TAB

2008-10-16 Thread Jim Meyering
One of the new (binary) .ico files has a TAB just before a newline,
and that triggers a make syntax-check failure.
This gives all *.ico files a free pass.
(also sorted the file)

From 11096480dfb54fc179b3854d3f81739f2d2ef273 Mon Sep 17 00:00:00 2001
From: Jim Meyering [EMAIL PROTECTED]
Date: Thu, 16 Oct 2008 15:05:16 +0200
Subject: [PATCH] build: exempt *.ico files from the trailing blank check

* .x-sc_trailing_blank: Add \.ico$ to the list.
---
 .x-sc_trailing_blank |5 +++--
 ChangeLog|5 +
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/.x-sc_trailing_blank b/.x-sc_trailing_blank
index 8b3ec89..de3369e 100644
--- a/.x-sc_trailing_blank
+++ b/.x-sc_trailing_blank
@@ -1,6 +1,7 @@
-\.png$
 \.fig$
 \.gif$
-^NEWS$
+\.ico$
+\.png$
 ^ChangeLog$
+^NEWS$
 ^docs/.*
diff --git a/ChangeLog b/ChangeLog
index 2aff530..322f837 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+Thu Oct 16 15:04:36 +0200 Jim Meyering [EMAIL PROTECTED]
+
+   build: exempt *.ico files from the trailing blank check
+   * .x-sc_trailing_blank: Add \.ico$ to the list.
+
 Wed Oct 16 14:03:00 CEST 2008 Chris Lalancette [EMAIL PROTECTED]

* make syntax-check was complaining that network_driver.c was
--
1.6.0.2.98.gc82e

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Jim Meyering
Chris Lalancette [EMAIL PROTECTED] wrote:
 Daniel P. Berrange wrote:
 I know you're just replicating the existing code, but both these functions 
 can
 be killed off and replaced with auto-generated code from our enum support
 macros

 VIR_ENUM_IMPL(virStorageBackendDiskLabel,
   VIR_STORAGE_POOL_DISK_LAST,
   dos, dvh, gpt, mac,
   bsd, pc98, sun, lvm2
   unknown)

 And in the header file just

 VIR_ENUM_DECL(virStorageBackendDiskLabel)

 I've implemented this, and there was quite a bit of fallout from it.  In
 particular, I had to change the signature of poolOptions.formatToString and
 formatFromString, which required a bunch of changes elsewhere in the storage
 code.  The good news is that we remove quite a bit of very similar hand 
 crafted
 code, so this patch actually removes more code than it adds.  Tested by me as
 before.

Lots of good fallout.
ACK with a tiny tweak:

 Chris Lalancette
 Index: src/storage_backend.c
 ===
 RCS file: /data/cvs/libvirt/src/storage_backend.c,v
 retrieving revision 1.21
 diff -u -r1.21 storage_backend.c
 --- a/src/storage_backend.c   5 Sep 2008 12:03:45 -   1.21
 +++ b/src/storage_backend.c   16 Oct 2008 12:31:23 -
 @@ -60,6 +60,11 @@
  #include storage_backend_fs.h
  #endif

 +VIR_ENUM_IMPL(virStorageBackendPartTable,
 +  VIR_STORAGE_POOL_DISK_LAST,
 +  unknown, dos, dvh, gpt,
 +  mac, bsd, pc98, sun, lvm2);
 +
  static virStorageBackendPtr backends[] = {
  #if WITH_STORAGE_DIR
  virStorageBackendDirectory,
 @@ -192,6 +197,30 @@
  return ret;
  }

 +const struct diskType const disk_types[] = {

s/const/static/

 +{ VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CUL },
 +{ VIR_STORAGE_POOL_DISK_GPT,  0x200, 8, 0x5452415020494645UL },
 +{ VIR_STORAGE_POOL_DISK_DVH,  0x0,   4, 0x41A9E50BUL },
 +{ VIR_STORAGE_POOL_DISK_MAC,  0x0,   2, 0x5245UL },
 +{ VIR_STORAGE_POOL_DISK_BSD,  0x40,  4, 0x82564557UL },
 +{ VIR_STORAGE_POOL_DISK_SUN,  0x1fc, 2, 0xBEDAUL },

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Suggestion on commit rules when fixing breakages

2008-10-16 Thread Daniel Veillard
  As the number of compilation options and platform grows, it gets
more difficult for a commiter to always ensure one chunk of code
won't give a problem in a different situation. To try to lower the
cost of maintaining the protability I would suggest the following
rule for commit:
  - if a recently commited patch breaks compilation on a platform
or for a given driver then it's fine to commit a minimal fix
directly without getting the review feedback first
  - similary if make check or make syntax-chek breaks, if there is
an obvious fix, it's fine to commit immediately
Note that this would remove the need to send the patch to the list
anyway (or tell what the fix was if trivial). This doesn't either
remove the rule that 'make check syntax-check' should pass before
commiting anything, and obviously the existing review process is still
needed t for anything which is not a trivial fix breaking make or
checks.

  I guess it makes sense to minimize disruption for those working on
head and lower the time needed to get those fix in for those who catch
and fix them ;-)
  Opinions ?

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | 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] fix make syntax-check vs. new .ico file with trailing TAB

2008-10-16 Thread Daniel Veillard
On Thu, Oct 16, 2008 at 03:08:47PM +0200, Jim Meyering wrote:
 One of the new (binary) .ico files has a TAB just before a newline,
 and that triggers a make syntax-check failure.
 This gives all *.ico files a free pass.

  I would not venture trying to guess if TAB could/should be converted
in ico files, skipping sounds just fine, +1

  A typical example for the suggested rule I just posted of pushing
immediately to CVS to keep 'make syntax-check' working

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | 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] Suggestion on commit rules when fixing breakages

2008-10-16 Thread Daniel Berrange
On Thu, Oct 16, 2008 at 03:15:08PM +0200, Daniel Veillard wrote:
   As the number of compilation options and platform grows, it gets
 more difficult for a commiter to always ensure one chunk of code
 won't give a problem in a different situation. To try to lower the
 cost of maintaining the protability I would suggest the following
 rule for commit:
   - if a recently commited patch breaks compilation on a platform
 or for a given driver then it's fine to commit a minimal fix
 directly without getting the review feedback first
   - similary if make check or make syntax-chek breaks, if there is
 an obvious fix, it's fine to commit immediately
 Note that this would remove the need to send the patch to the list
 anyway (or tell what the fix was if trivial). This doesn't either
 remove the rule that 'make check syntax-check' should pass before
 commiting anything, and obviously the existing review process is still
 needed t for anything which is not a trivial fix breaking make or
 checks.
 
   I guess it makes sense to minimize disruption for those working on
 head and lower the time needed to get those fix in for those who catch
 and fix them ;-)
   Opinions ?

Yes, this is reasonable idea - its crazy to wait for ACKs on the list
to fix 1-2 line typos in the code which break compilation.

I'll also remind all those with commit access that you should run be
running autogen.sh or configure with --enable-compile-warnings=error
which adds -Werror to compile flags, so no warnings get missed.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] Suggestion on commit rules when fixing breakages

2008-10-16 Thread Daniel Veillard
On Thu, Oct 16, 2008 at 03:15:08PM +0200, Daniel Veillard wrote:
   As the number of compilation options and platform grows, it gets
 more difficult for a commiter to always ensure one chunk of code
 won't give a problem in a different situation. To try to lower the
 cost of maintaining the protability I would suggest the following
 rule for commit:
   - if a recently commited patch breaks compilation on a platform
 or for a given driver then it's fine to commit a minimal fix
 directly without getting the review feedback first
   - similary if make check or make syntax-chek breaks, if there is
 an obvious fix, it's fine to commit immediately
 Note that this would remove the need to send the patch to the list

  would *not*  sigh !

 anyway (or tell what the fix was if trivial). This doesn't either
 remove the rule that 'make check syntax-check' should pass before

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | 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] Suggestion on commit rules when fixing breakages

2008-10-16 Thread Jim Meyering
Daniel Berrange [EMAIL PROTECTED] wrote:
 On Thu, Oct 16, 2008 at 03:15:08PM +0200, Daniel Veillard wrote:
...
   I guess it makes sense to minimize disruption for those working on
 head and lower the time needed to get those fix in for those who catch
 and fix them ;-)
   Opinions ?

 Yes, this is reasonable idea - its crazy to wait for ACKs on the list
 to fix 1-2 line typos in the code which break compilation.

I think so too, and have just committed this:

build: exempt *.ico files from the trailing blank check
* .x-sc_trailing_blank: Add \.ico$ to the list.

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] Suggestion on commit rules when fixing breakages

2008-10-16 Thread Atsushi SAKAI
Hi, Daniel

 This proposal seems approved.
Does this proposal message goes to somewhere in libvirt document?

Thanks
Atsushi SAKAI


Daniel Veillard [EMAIL PROTECTED] wrote:

   As the number of compilation options and platform grows, it gets
 more difficult for a commiter to always ensure one chunk of code
 won't give a problem in a different situation. To try to lower the
 cost of maintaining the protability I would suggest the following
 rule for commit:
   - if a recently commited patch breaks compilation on a platform
 or for a given driver then it's fine to commit a minimal fix
 directly without getting the review feedback first
   - similary if make check or make syntax-chek breaks, if there is
 an obvious fix, it's fine to commit immediately
 Note that this would remove the need to send the patch to the list
 anyway (or tell what the fix was if trivial). This doesn't either
 remove the rule that 'make check syntax-check' should pass before
 commiting anything, and obviously the existing review process is still
 needed t for anything which is not a trivial fix breaking make or
 checks.
 
   I guess it makes sense to minimize disruption for those working on
 head and lower the time needed to get those fix in for those who catch
 and fix them ;-)
   Opinions ?
 
 Daniel
 
 -- 
 Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
 [EMAIL PROTECTED]  | 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


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Committed a fix for building with -Werror

2008-10-16 Thread Chris Lalancette
I just committed this:

+Thu Oct 16 15:41:00 CEST 2008 Chris Lalancette [EMAIL PROTECTED]
+   * Compiling with -Werror showed a possible use before initialization
+   in src/qemu_driver.c.  Make sure to initialize the olddisk ptr to
+   NULL.
+

Index: src/qemu_driver.c
===
RCS file: /data/cvs/libvirt/src/qemu_driver.c,v
retrieving revision 1.135
diff -u -r1.135 qemu_driver.c
--- src/qemu_driver.c   13 Oct 2008 16:46:28 -  1.135
+++ src/qemu_driver.c   16 Oct 2008 13:42:17 -
@@ -2401,6 +2401,7 @@
 return -1;
 }

+origdisk = NULL;
 newdisk = dev-data.disk;
 for (i = 0 ; i  vm-def-ndisks ; i++) {
 if (vm-def-disks[i]-bus == newdisk-bus 


-- 
Chris Lalancette

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] Suggestion on commit rules when fixing breakages

2008-10-16 Thread Daniel Veillard
On Thu, Oct 16, 2008 at 10:41:02PM +0900, Atsushi SAKAI wrote:
 Hi, Daniel
 
  This proposal seems approved.
 Does this proposal message goes to somewhere in libvirt document?

  Well typos fixes for documentation and code comments could be managed
in the same way. It doesn't break builds, and very few people ;-) seems
to chase them. As long as the patch is still sent to the list, that
would be fine by me !

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | 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 1/4: More generic MAC address handling

2008-10-16 Thread Jim Meyering
Daniel P. Berrange [EMAIL PROTECTED] wrote:
 This patch improves the MAC address handling.

 Currently our XML parser auto-generates a MAC addres using the KVM vendor
 prefix. This isn't much use for other drivers. This patch addresses this:

  - Stores each driver's vendor prefix in the capability object
  - Changes domain parser to use the per-driver vendor prefix for
generating mac addresses
  - Adds more utility methods to util.c for parsing/generating/formatting
MAC addresses
  - Updates each driver to record its vendor prefix for MAC address.

This all looks fine, but I have a question about the moved code
that makes up the new virGenerateMacAddr function:

 +void virGenerateMacAddr(const unsigned char *prefix,
 +unsigned char *addr)
 +{
 +addr[0] = prefix[0];
 +addr[1] = prefix[1];
 +addr[2] = prefix[2];
 +addr[3] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
 +addr[4] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
 +addr[5] = 1 + (int)(256*(rand()/(RAND_MAX+1.0)));
 +}

I presume the intent is to generated numbers in 0..255,
but that code generates them in 1..256 and relies on the
truncate-to-8-bit-unsigned-char to map back to 0..255.

Why are those 1 + there?
It doesn't seem to hurt, but doesn't make sense (to me), either.
Removing them would not change the results.

Also, unless there's a guarantee that the random number state is
initialized elsewhere, it should be initialized here, like it was in
the now-removed xenXMAutoAssignMac function.

   srand((unsigned)time(NULL));

Or maybe just initialize it once at start-up?

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH 4/4: support FS template config

2008-10-16 Thread Jim Meyering
Daniel P. Berrange [EMAIL PROTECTED] wrote:
 The root filesystem for an openvz guest is defined from a template name.
 We support this when creating a new guest, but never include this info
 when dumping the XML. Thsi patch addresses this problem by reading the
 OSTEMPLATE config parameter

 diff -r e0c166ce24bd src/openvz_conf.c
 --- a/src/openvz_conf.c   Tue Oct 14 15:46:24 2008 +0100
 +++ b/src/openvz_conf.c   Tue Oct 14 15:49:41 2008 +0100
 @@ -308,6 +308,47 @@ error:
  }


 +static int
 +openvzReadFSConf(virConnectPtr conn,
 + virDomainDefPtr def,
 + int veid) {
 +int ret;
 +virDomainFSDefPtr fs;

This needs to be initialized here or in the 'if' block.
Otherwise, it will be freed uninitialized upon read failure:

   virDomainFSDefPtr fs = NULL;

 +char temp[4096];
 +
 +ret = openvzReadConfigParam(veid, OSTEMPLATE, temp, sizeof(temp));
 +if (ret  0) {
 +openvzError(conn, VIR_ERR_INTERNAL_ERROR,
 +_(Cound not read 'OSTEMPLATE' from config for container 
 %d),
 +veid);
 +goto error;
 +} else if (ret  0) {
 +if (VIR_ALLOC(fs)  0)
 +goto no_memory;
 +
 +fs-type = VIR_DOMAIN_FS_TYPE_TEMPLATE;
 +fs-src = strdup(temp);
 +fs-dst = strdup(/);
 +
 +if (fs-src == NULL || fs-dst == NULL)
 +goto no_memory;
 +
 +if (VIR_REALLOC_N(def-fss, def-nfss + 1)  0)
 +goto no_memory;
 +def-fss[def-nfss++] = fs;
 +fs = NULL;
 +
 +}
 +
 +return 0;
 +no_memory:
 +openvzError(conn, VIR_ERR_NO_MEMORY, NULL);
 +error:
 +virDomainFSDefFree(fs);
 +return -1;
 +}
 +
 +
  /* Free all memory associated with a openvz_driver structure */
  void
  openvzFreeDriver(struct openvz_driver *driver)
 @@ -393,6 +434,7 @@ int openvzLoadDomains(struct openvz_driv
  /* XXX load rest of VM config data  */

  openvzReadNetworkConf(NULL, dom-def, veid);
 +openvzReadFSConf(NULL, dom-def, veid);

  if (VIR_REALLOC_N(driver-domains.objs,
driver-domains.count + 1)  0)

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Daniel P. Berrange
On Thu, Oct 16, 2008 at 11:52:48AM +0200, Chris Lalancette wrote:
 Chris Lalancette wrote:
  To support LVM partitioning in oVirt, one of the things we need is the 
  ability
  to tell what kind of label is currently on a block device.  Here, a 'label' 
  is
  used in the same sense that it is used in parted; namely, it defines which 
  kind
  of partition table is on the disk, whether it be DOS, LVM2, SUN, BSD, etc.  
  Note
  that this is different than the partition type; those are things like Linux,
  FAT16, FAT32, etc.
 
 Updated patch based on jim's feedback.  I did not remove the pc98 type; if
 everyone else thinks it's a good idea, I'll remove it, but it is part of the 
 ABI
 in some sense.  Otherwise, I followed all of jim's suggestions.

I think we can keep it for completeness, since partd supports it, it is
conceivable (though unlikely) that we can see it and its not a real
burden to keep it.

 ===
 RCS file: /data/cvs/libvirt/src/storage_backend.c,v
 retrieving revision 1.21
 diff -u -r1.21 storage_backend.c
 --- src/storage_backend.c 5 Sep 2008 12:03:45 -   1.21
 +++ src/storage_backend.c 16 Oct 2008 09:49:01 -
 @@ -192,6 +192,30 @@
  return ret;
  }
  
 +struct diskType disk_types[] = {

This can be const const - surprised Jim didn't catch this :-)

 +{ lvm2, VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CUL },
 +{ gpt,  VIR_STORAGE_POOL_DISK_GPT,  0x200, 8, 0x5452415020494645UL },
 +{ dvh,  VIR_STORAGE_POOL_DISK_DVH,  0x0,   4, 0x41A9E50BUL },
 +{ mac,  VIR_STORAGE_POOL_DISK_MAC,  0x0,   2, 0x5245UL },
 +{ bsd,  VIR_STORAGE_POOL_DISK_BSD,  0x40,  4, 0x82564557UL },
 +{ sun,  VIR_STORAGE_POOL_DISK_SUN,  0x1fc, 2, 0xBEDAUL },
 +/*
 + * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), 
 so
 + * we can't use that.  At the moment I'm relying on the dummy IPL
 + * bootloader data that comes from parted.  Luckily, the chances of 
 running
 + * into a pc98 machine running libvirt are approximately nil.
 + */
 +/*{ pc98, 0x1fe, 2, 0xAA55UL },*/
 +{ pc98, VIR_STORAGE_POOL_DISK_PC98, 0x0,   8, 0x314C504900CBUL },
 +/*
 + * NOTE: the order is important here; some other disk types (like GPT and
 + * and PC98) also have 0x55AA at this offset.  For that reason, the DOS
 + * one must be the last one.
 + */
 +{ dos,  VIR_STORAGE_POOL_DISK_DOS,  0x1fe, 2, 0xAA55UL },
 +{ NULL,   -1, 0x0,   0, 0x0UL },
 +};
 +

Remove the strings from here - they're not required for the probing - only
the constant it needed. The string-ification of the types is better handled
by our enum support

 +int
 +virStorageBackendDiskLabelFormatFromString(virConnectPtr conn 
 ATTRIBUTE_UNUSED,
 +   const char *format) {
 +int i;
 +
 +if (format == NULL)
 +return VIR_STORAGE_POOL_DISK_UNKNOWN;
 +
 +for (i = 0; disk_types[i].name != NULL; i++) {
 +if (STREQ(format, disk_types[i].name))
 + return disk_types[i].part_table_type;
 +}
 +
 +return VIR_STORAGE_POOL_DISK_UNKNOWN;
 +}
 +
 +const char *
 +virStorageBackendDiskLabelFormatToString(virConnectPtr conn ATTRIBUTE_UNUSED,
 + int format) {
 +int i;
 +
 +for (i = 0; disk_types[i].name != NULL; i++) {
 +if (format == disk_types[i].part_table_type)
 +return disk_types[i].name;
 +}
 +
 +return unknown;
 +}

I know you're just replicating the existing code, but both these functions can
be killed off and replaced with auto-generated code from our enum support
macros

VIR_ENUM_IMPL(virStorageBackendDiskLabel, 
  VIR_STORAGE_POOL_DISK_LAST,
  dos, dvh, gpt, mac,
  bsd, pc98, sun, lvm2
  unknown)

And in the header file just

VIR_ENUM_DECL(virStorageBackendDiskLabel)

  
  #ifndef __MINGW32__
  /*
 Index: src/storage_backend.h
 ===
 RCS file: /data/cvs/libvirt/src/storage_backend.h,v
 retrieving revision 1.7
 diff -u -r1.7 storage_backend.h
 --- src/storage_backend.h 2 Sep 2008 14:15:42 -   1.7
 +++ src/storage_backend.h 16 Oct 2008 09:49:01 -
 @@ -56,6 +56,29 @@
  VIR_STORAGE_BACKEND_POOL_SOURCE_NAME= (14),
  };
  
 +enum partTableType {
 +VIR_STORAGE_POOL_DISK_DOS = 1,
 +VIR_STORAGE_POOL_DISK_DVH,
 +VIR_STORAGE_POOL_DISK_GPT,
 +VIR_STORAGE_POOL_DISK_MAC,
 +VIR_STORAGE_POOL_DISK_BSD,
 +VIR_STORAGE_POOL_DISK_PC98,
 +VIR_STORAGE_POOL_DISK_SUN,
 +VIR_STORAGE_POOL_DISK_LVM2,
 +/* the unknown disk is 1 billion (and not, for instance, -1), to make
 +   sure it doesn't run afoul of error checking */
 +VIR_STORAGE_POOL_DISK_UNKNOWN = 1 * 1024 * 1024 * 1024,

I don't understand why it has to be 1-billion ? For the enum 

Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Chris Lalancette
Daniel P. Berrange wrote:
 I know you're just replicating the existing code, but both these functions can
 be killed off and replaced with auto-generated code from our enum support
 macros
 
 VIR_ENUM_IMPL(virStorageBackendDiskLabel, 
   VIR_STORAGE_POOL_DISK_LAST,
   dos, dvh, gpt, mac,
   bsd, pc98, sun, lvm2
   unknown)
 
 And in the header file just
 
 VIR_ENUM_DECL(virStorageBackendDiskLabel)

I've implemented this, and there was quite a bit of fallout from it.  In
particular, I had to change the signature of poolOptions.formatToString and
formatFromString, which required a bunch of changes elsewhere in the storage
code.  The good news is that we remove quite a bit of very similar hand crafted
code, so this patch actually removes more code than it adds.  Tested by me as
before.

-- 
Chris Lalancette
Index: src/storage_backend.c
===
RCS file: /data/cvs/libvirt/src/storage_backend.c,v
retrieving revision 1.21
diff -u -r1.21 storage_backend.c
--- a/src/storage_backend.c	5 Sep 2008 12:03:45 -	1.21
+++ b/src/storage_backend.c	16 Oct 2008 12:31:23 -
@@ -60,6 +60,11 @@
 #include storage_backend_fs.h
 #endif
 
+VIR_ENUM_IMPL(virStorageBackendPartTable,
+  VIR_STORAGE_POOL_DISK_LAST,
+  unknown, dos, dvh, gpt,
+  mac, bsd, pc98, sun, lvm2);
+
 static virStorageBackendPtr backends[] = {
 #if WITH_STORAGE_DIR
 virStorageBackendDirectory,
@@ -192,6 +197,30 @@
 return ret;
 }
 
+const struct diskType const disk_types[] = {
+{ VIR_STORAGE_POOL_DISK_LVM2, 0x218, 8, 0x31303020324D564CUL },
+{ VIR_STORAGE_POOL_DISK_GPT,  0x200, 8, 0x5452415020494645UL },
+{ VIR_STORAGE_POOL_DISK_DVH,  0x0,   4, 0x41A9E50BUL },
+{ VIR_STORAGE_POOL_DISK_MAC,  0x0,   2, 0x5245UL },
+{ VIR_STORAGE_POOL_DISK_BSD,  0x40,  4, 0x82564557UL },
+{ VIR_STORAGE_POOL_DISK_SUN,  0x1fc, 2, 0xBEDAUL },
+/*
+ * NOTE: pc98 is funky; the actual signature is 0x55AA (just like dos), so
+ * we can't use that.  At the moment I'm relying on the dummy IPL
+ * bootloader data that comes from parted.  Luckily, the chances of running
+ * into a pc98 machine running libvirt are approximately nil.
+ */
+/*{ 0x1fe, 2, 0xAA55UL },*/
+{ VIR_STORAGE_POOL_DISK_PC98, 0x0,   8, 0x314C504900CBUL },
+/*
+ * NOTE: the order is important here; some other disk types (like GPT and
+ * and PC98) also have 0x55AA at this offset.  For that reason, the DOS
+ * one must be the last one.
+ */
+{ VIR_STORAGE_POOL_DISK_DOS,  0x1fe, 2, 0xAA55UL },
+{ -1, 0x0,   0, 0x0UL },
+};
+
 int
 virStorageBackendUpdateVolInfoFD(virConnectPtr conn,
  virStorageVolDefPtr vol,
@@ -245,6 +274,41 @@
 if (withCapacity) vol-capacity = end;
 }
 
+/* make sure to set the target format unknown to begin with */
+vol-target.format = VIR_STORAGE_POOL_DISK_UNKNOWN;
+
+if (S_ISBLK(sb.st_mode)) {
+off_t start;
+int i;
+unsigned char buffer[1024];
+ssize_t bytes;
+
+start = lseek(fd, 0, SEEK_SET);
+if (start  0) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+  _(cannot seek to beginning of file '%s':%s),
+  vol-target.path, strerror(errno));
+return -1;
+}
+bytes = saferead(fd, buffer, sizeof(buffer));
+if (bytes  0) {
+virStorageReportError(conn, VIR_ERR_INTERNAL_ERROR,
+  _(cannot read beginning of file '%s':%s),
+  vol-target.path, strerror(errno));
+return -1;
+}
+
+for (i = 0; disk_types[i].part_table_type != -1; i++) {
+if (disk_types[i].offset + disk_types[i].length  bytes)
+continue;
+if (memcmp(buffer+disk_types[i].offset, disk_types[i].magic,
+disk_types[i].length) == 0) {
+vol-target.format = disk_types[i].part_table_type;
+break;
+}
+}
+}
+
 vol-target.perms.mode = sb.st_mode;
 vol-target.perms.uid = sb.st_uid;
 vol-target.perms.gid = sb.st_gid;
Index: src/storage_backend.h
===
RCS file: /data/cvs/libvirt/src/storage_backend.h,v
retrieving revision 1.7
diff -u -r1.7 storage_backend.h
--- a/src/storage_backend.h	2 Sep 2008 14:15:42 -	1.7
+++ b/src/storage_backend.h	16 Oct 2008 12:31:23 -
@@ -26,18 +26,14 @@
 
 #include libvirt/libvirt.h
 #include storage_conf.h
+#include util.h
 
 
-typedef const char *(*virStorageVolFormatToString)(virConnectPtr conn,
-   int format);
-typedef int (*virStorageVolFormatFromString)(virConnectPtr conn,
-   

Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Daniel P. Berrange
On Thu, Oct 16, 2008 at 03:47:26PM +0200, Chris Lalancette wrote:
 Daniel P. Berrange wrote:
  diff -u -r1.15 storage_backend_fs.c
  --- a/src/storage_backend_fs.c 13 Oct 2008 16:46:29 -  1.15
  +++ b/src/storage_backend_fs.c 16 Oct 2008 12:31:23 -
  @@ -48,7 +48,8 @@
   #include xml.h
   
   enum {
  -VIR_STORAGE_POOL_FS_AUTO = 0,
  +VIR_STORAGE_POOL_FS_UNKNOWN = 0,
  +VIR_STORAGE_POOL_FS_AUTO = 1,
  
  This shouldn't be added - automatic is intended to be default.
  
   
   enum {
  -VIR_STORAGE_VOL_RAW,
  +VIR_STORAGE_VOL_UNKNOWN = 0,
  +VIR_STORAGE_VOL_RAW = 1,
  
 
 OK, hopefully last try.  Fixed the above two errors and made the disk_type 
 array
 static as suggested by Jim.
 
  
  enum {
 -VIR_STORAGE_POOL_NETFS_AUTO = 0,
 +VIR_STORAGE_POOL_NETFS_UNKNOWN = 0,
 +VIR_STORAGE_POOL_NETFS_AUTO = 1,
  VIR_STORAGE_POOL_NETFS_NFS,
 +VIR_STORAGE_POOL_NETFS_LAST,
  };

I missed that one last time around - no need for unknown there. Just
fix that when committing - the rest is fine now.


Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH]: Export the 'label' on block devices

2008-10-16 Thread Jim Meyering
Chris Lalancette [EMAIL PROTECTED] wrote:
 Daniel P. Berrange wrote:
 diff -u -r1.15 storage_backend_fs.c
 --- a/src/storage_backend_fs.c  13 Oct 2008 16:46:29 -  1.15
 +++ b/src/storage_backend_fs.c  16 Oct 2008 12:31:23 -
 @@ -48,7 +48,8 @@
  #include xml.h

  enum {
 -VIR_STORAGE_POOL_FS_AUTO = 0,
 +VIR_STORAGE_POOL_FS_UNKNOWN = 0,
 +VIR_STORAGE_POOL_FS_AUTO = 1,

 This shouldn't be added - automatic is intended to be default.


  enum {
 -VIR_STORAGE_VOL_RAW,
 +VIR_STORAGE_VOL_UNKNOWN = 0,
 +VIR_STORAGE_VOL_RAW = 1,


 OK, hopefully last try.  Fixed the above two errors and made the disk_type 
 array
 static as suggested by Jim.

ACK.
Looks fine, applied, and passed smoke test:
make check  make syntax-check

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: Experimental User Mode Linux driver for libvirt

2008-10-16 Thread Daniel Veillard
On Thu, Oct 16, 2008 at 12:42:17PM +0100, Daniel P. Berrange wrote:
[...]
 Like QEMU, UML has an admin monitor console. It is a little different
 though, using a UNIX domain socket, in datagram mode, sending command
 strings back  forth. This actaully makes it a little easier to deal
 with in libvirt. User mode linux has a uml_mconsole command line tool
 to interact with it, but I directly implement the socket support in
 libvirt instead, so this driver does not (yet) have any dependancy on
 the UML utilities.
 
 The driver is largely a clone of the QEMU driver, replacing the bit which
 builds the command line argv, and all code dealing with the QEMU monitor.
 Since the monitor socket is datagram based, we can't make use of it for
 detecting VM shutdown  as we do with QEMU/LXC. So instead, I make use of
 inotify, to monitor $HOME/.uml which is populated witha directory for
 each VM. When we see a directory created, libvirt marks the corresponding
 VM as running, finds it PID  probes the monitor for the PTY config. When
 we see a directory deleted, libvirt makes the VM as shutoff and frees any
 resources its holding. This actually works very nicely  simply.

  Seems then that this driver could be made restarteable easilly in case
of libvirtd shutdown/restart, right ? Since everything is available from
the filesystem and there is no tight coupling between libvirtd and
the uml process, it even seems a starting libvirtd would be able to
discover and manage uml instances which were not started with libvirt,
right ?

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | 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]: Export the 'label' on block devices

2008-10-16 Thread Chris Lalancette
Daniel P. Berrange wrote:
  enum {
 -VIR_STORAGE_POOL_NETFS_AUTO = 0,
 +VIR_STORAGE_POOL_NETFS_UNKNOWN = 0,
 +VIR_STORAGE_POOL_NETFS_AUTO = 1,
  VIR_STORAGE_POOL_NETFS_NFS,
 +VIR_STORAGE_POOL_NETFS_LAST,
  };
 
 I missed that one last time around - no need for unknown there. Just
 fix that when committing - the rest is fine now.

Committed with this last change in place.

-- 
Chris Lalancette

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH] Thu Oct 16 17:07:10 +0200 Jim Meyering [EMAIL PROTECTED]

2008-10-16 Thread Jim Meyering
The output of a failed po-check rule (part of make syntax-check)
was unnecessarily cryptic.  Here's the fix that I've already applied
in upstream coreutils:

build: when po-check fails, say why and suggest a fix

* Makefile.maint (po-check): Before, when this check failed, it just
spat out a diff mentioning two temporary files.  Now, it tells you
what's wrong and suggests a fix with a patch using the name of the
affected file (rather than temporary file names) in the diff output.

diff --git a/Makefile.maint b/Makefile.maint
index 03800f8..4920112 100644
--- a/Makefile.maint
+++ b/Makefile.maint
@@ -466,8 +466,12 @@ m4-check:
   { echo 'Makefile.maint: quote the first arg to AC_DEFUN' 12; \
   exit 1; } || :

+fix_po_file_diag = \
+'you have changed the set of files with translatable diagnostics;\n\
+apply the above patch\n'
+
 # Verify that all source files using _() are listed in po/POTFILES.in.
-# FIXME: don't hard-code file names below; use a more general mechanism.
+po_file = po/POTFILES.in
 po-check:
@if test -f po/POTFILES.in; then
\
  grep -E -v '^(#|$$)' po/POTFILES.in   \
@@ -482,13 +486,14 @@ po-check:
*.[ch]) \
  base=`expr  $$file : ' \(.*\)\..'`; \
  { test -f $$base.l || test -f $$base.y; }  continue;;   \
-   *) continue;;   \
+   *) continue;;   \
esac;   \
files=$$files $$file; \
  done; \
  grep -E -l '\b(N?_|gettext *)\([^)]*(|$$)' $$files  \
| sort -u  [EMAIL PROTECTED];  
\
- diff -u [EMAIL PROTECTED] [EMAIL PROTECTED] || exit 1;
\
+ diff -u -L $(po_file) -L $(po_file) [EMAIL PROTECTED] [EMAIL 
PROTECTED]   \
+   || { printf '$(ME): '$(fix_po_file_diag) 12; exit 1; };   \
  rm -f [EMAIL PROTECTED] [EMAIL PROTECTED];
\
fi

--
1.6.0.2.98.gc82e

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains

2008-10-16 Thread Daniel P. Berrange
On Fri, Oct 03, 2008 at 08:40:24AM -0700, Dan Smith wrote:
 This patch adds code to the controller to set up a cgroup named after the
 domain name, set the memory limit, and restrict devices.  It also
 adds bits to lxc_driver to properly clean up the cgroup on domain death.

The device whitelisting is all very nice, but we completely forgot / ignored
the fact that there's nothing stopping a container mounting the cgroups 
device controller and giving itself the device access we just took away :-)

The kernel code says

/*
 * Modify the whitelist using allow/deny rules.
 * CAP_SYS_ADMIN is needed for this.  It's at least separate from CAP_MKNOD
 * so we can give a container CAP_MKNOD to let it create devices but not
 * modify the whitelist.
 * It seems likely we'll want to add a CAP_CONTAINER capability to allow
 * us to also grant CAP_SYS_ADMIN to containers without giving away the
 * device whitelist controls, but for now we'll stick with CAP_SYS_ADMIN
 *
 * Taking rules away is always allowed (given CAP_SYS_ADMIN).  Granting
 * new access is only allowed if you're in the top-level cgroup, or your
 * parent cgroup has the access you're asking for.
 */


So, looks like we need to explicitly set the capabilities of containers
to either mask out CAP_SYS_ADMIN from libvirtd's set, or construct an
explicit capability whitelist

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH: Experimental User Mode Linux driver for libvirt

2008-10-16 Thread Daniel P. Berrange
On Thu, Oct 16, 2008 at 04:59:24PM +0200, Daniel Veillard wrote:
 On Thu, Oct 16, 2008 at 12:42:17PM +0100, Daniel P. Berrange wrote:
 [...]
  Like QEMU, UML has an admin monitor console. It is a little different
  though, using a UNIX domain socket, in datagram mode, sending command
  strings back  forth. This actaully makes it a little easier to deal
  with in libvirt. User mode linux has a uml_mconsole command line tool
  to interact with it, but I directly implement the socket support in
  libvirt instead, so this driver does not (yet) have any dependancy on
  the UML utilities.
  
  The driver is largely a clone of the QEMU driver, replacing the bit which
  builds the command line argv, and all code dealing with the QEMU monitor.
  Since the monitor socket is datagram based, we can't make use of it for
  detecting VM shutdown  as we do with QEMU/LXC. So instead, I make use of
  inotify, to monitor $HOME/.uml which is populated witha directory for
  each VM. When we see a directory created, libvirt marks the corresponding
  VM as running, finds it PID  probes the monitor for the PTY config. When
  we see a directory deleted, libvirt makes the VM as shutoff and frees any
  resources its holding. This actually works very nicely  simply.
 
   Seems then that this driver could be made restarteable easilly in case
 of libvirtd shutdown/restart, right ? Since everything is available from
 the filesystem and there is no tight coupling between libvirtd and
 the uml process, it even seems a starting libvirtd would be able to
 discover and manage uml instances which were not started with libvirt,
 right ?

Yes, this was written with restartability in mind - all I need to add is
saving of the live XML config, and re-connecting to the monitor socket,
and it'll all survive daemon restarts - this was the core motivation
for completely daemonizing the child processes  using inotify to monitor
their birth  death.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains

2008-10-16 Thread Dan Smith
DB The device whitelisting is all very nice, but we completely forgot
DB / ignored the fact that there's nothing stopping a container
DB mounting the cgroups device controller and giving itself the
DB device access we just took away :-)

Ah, interesting.

DB So, looks like we need to explicitly set the capabilities of
DB containers to either mask out CAP_SYS_ADMIN from libvirtd's set,
DB or construct an explicit capability whitelist

Yeah, I guess so.  I'll start looking into this :)

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: [EMAIL PROTECTED]

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] PATCH 3/4: SUpport bridge config for openvz

2008-10-16 Thread Evgeniy Sokolov

This implements support for bridge configs in openvz following the rules
set out in

http://wiki.openvz.org/Virtual_Ethernet_device#Making_a_bridged_veth-device_persistent

This simply requires that the admin has created /etc/vz/vznetctl.conf
containing

  #!/bin/bash
  EXTERNAL_SCRIPT=/usr/sbin/vznetaddbr


For openvz = 3.0.22, we have to manually re-write the NETIF line to
add the bridge config parameter. 
It is alternative, but more flexible way. It require simple modification 
of vznetaddbr.

Common scenario is to add VZHOSTBR=bridge if to config.
For newer openvz we can simply pass

the bridge name on the commnand line to --netif_add.

Older openvz also requires that the admin install /usr/sbin/vznetaddbr
since it is not available out of the box

Daniel





+
+while(1) {
+if (openvz_readline(fd, line, sizeof(line)) = 0)
+break;
+
+if (!STRPREFIX(line, param)) {

need to check for '='.
Currently, if you will search for 'NETIF', you can find any string with 
such prefix. example 'NETIFOTHERPARAM'

+if (safewrite(temp_fd, line, strlen(line)) !=
+strlen(line))
+goto error;
+}
+}
+
 /*



+
+if (!(opt = virBufferContentAndReset(buf)))
+goto no_memory;
+
 ADD_ARG_LIT(opt) ;

Need to free opt

-}else if (net-type == VIR_DOMAIN_NET_TYPE_ETHERNET 
+} else if (net-type == VIR_DOMAIN_NET_TYPE_ETHERNET 
   net-data.ethernet.ipaddr != NULL) {



+static int
+openvzDomainSetNetworkConfig(virConnectPtr conn,
+ virDomainDefPtr def)
+{
+unsigned int i;
+virBuffer buf = VIR_BUFFER_INITIALIZER;
+char *param;
+struct openvz_driver *driver = (struct openvz_driver *) conn-privateData;
+
+for (i = 0 ; i  def-nnets ; i++) {
+if (driver-version  VZCTL_BRIDGE_MIN_VERSION  i  0)
+virBufferAddLit(buf, ;);

Need to check that network is bridge.
In other case we will have NETIF='some params'

+
+if (openvzDomainSetNetwork(conn, def-name, def-nets[i], buf)  0) {
+openvzError(conn, VIR_ERR_INTERNAL_ERROR,
+%s, _(Could not configure network));
+goto exit;
+}
+}
+


--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] libvirt-qpid

2008-10-16 Thread Ian Main
On Mon, 13 Oct 2008 15:56:21 +0200
Daniel Veillard [EMAIL PROTECTED] wrote:

 On Wed, Sep 24, 2008 at 10:55:36AM -0700, Ian Main wrote:
  Once you have that set up, 'yum install libvirt-qpid python-qpid', and then
  run (each in their own terminals):
  
  qpidd --auth no
  libvirt-qpid (as root to auth with libvirt)
 
   I'm a little bit worried by the following:
 
   PID USER  PR  NI  VIRT  RES  SHR S %CPU %MEMTIME+  COMMAND
  9758 qpidd 20   0  273m 137m 1644 S 46.6  6.8 986:30.29 qpidd
 
  that's with just 3 nodes over a week with no traffic except the
 keeping the machines on the bus, the memory footprint of qpidd
 seems to be growing, very slowly but nonetheless continuously, and
 that's just with 3 nodes hooked with libvirt-qpid.
  is that a genuine leak in qpidd ? (qpidd-0.3.700546-1.fc9.x86_64)
  is that related to the policy to keep messages sent to the QPid bus ?
 
   I wonder. It also consumes some non-neglective amount of CPU as
 a background task.

I talked to the qpid folks about this today.  It's a bug one way or another.
They fixed one that may be causing this and there will be new RPMs out 
hopefully today.  If that doesn't do it it may have something to do with
how QMF is doing things.  If you still see this after the RPM upgrade
let me know.  I'll be trying it out too.  Thanks!

Ian

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] [PATCH 2 of 2] [LXC] Create and enter the cgroup before starting container process

2008-10-16 Thread Dan Smith
Without this, our container child doesn't actually end up in the cgroup,
and thus runs unrestricted.  Note that this does not address the container's
ability to mount cgroup and move itself into the parent namespace.

While making this change, it became clear that we need to allow access to
the entire range of pty devices for the container console to work.  This
patch adds that logic as well.

diff -r 48a9209668ab -r 4babf77ec518 src/lxc_container.h
--- a/src/lxc_container.h   Thu Oct 16 14:01:13 2008 -0700
+++ b/src/lxc_container.h   Thu Oct 16 14:01:13 2008 -0700
@@ -40,6 +40,8 @@
 #define LXC_DEV_MAJ_TTY 5
 #define LXC_DEV_MIN_CONSOLE 1
 
+#define LXC_DEV_MAJ_PTY 136
+
 int lxcContainerSendContinue(int control);
 
 int lxcContainerStart(virDomainDefPtr def,
diff -r 48a9209668ab -r 4babf77ec518 src/lxc_controller.c
--- a/src/lxc_controller.c  Thu Oct 16 14:01:13 2008 -0700
+++ b/src/lxc_controller.c  Thu Oct 16 14:01:13 2008 -0700
@@ -102,6 +102,10 @@
 if (rc != 0)
 goto out;
 }
+
+rc = virCgroupAllowDeviceMajor(cgroup, 'c', LXC_DEV_MAJ_PTY);
+if (rc != 0)
+goto out;
 
 rc = virCgroupAddTask(cgroup, getpid());
 out:
@@ -449,6 +453,9 @@
 goto cleanup;
 }
 
+if (lxcSetContainerResources(def)  0)
+goto cleanup;
+
 if ((container = lxcContainerStart(def,
nveths,
veths,
@@ -459,9 +466,6 @@
 control[1] = -1;
 
 if (lxcControllerMoveInterfaces(nveths, veths, container)  0)
-goto cleanup;
-
-if (lxcSetContainerResources(def)  0)
 goto cleanup;
 
 if (lxcContainerSendContinue(control[0])  0)

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] [PATCH 2 of 2] Use cgroup functions to set resource limits on LXC domains

2008-10-16 Thread Dan Smith
DB So if that source code comment is correct, all we need todo is set
DB a deny-all rule in that intermediate 'lxc' cgroup, and then
DB containers will not be able to get access back, even if they have
DB CAP_SYS_ADMIN

Even if I make the per-driver group have a deny-all policy, I can
still add arbitrary items to devices.allow and gain access from a
subgroup.  So, I think we're going to need to restrict CAP_SYS_ADMIN
if we really want isolation, but I'm not sure what else that is likely
to break.

-- 
Dan Smith
IBM Linux Technology Center
Open Hypervisor Team
email: [EMAIL PROTECTED]

--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


[libvirt] Question about supporting other hypervisor

2008-10-16 Thread Atsushi SAKAI
Hello,

I have a simple question raised yesterday.

CIM(Common Information Model) is a kind of good I/F
(to support VMware, Hyper-V and other platform).

As a CIM Provider, libvirt-cim is going on.
But for CIM Client, Is not going on.

Is there any reason for not supporting CIM on libvirt driver layer?
I am thinking about cim-xml driver (like remote driver) in libvirt.

Ref.
http://www.dmtf.org/standards/wbem
http://en.wikipedia.org/wiki/Web-Based_Enterprise_Management

Thanks
Atsushi SAKAI



--
Libvir-list mailing list
Libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list


Re: [libvirt] libvirt-qpid

2008-10-16 Thread Daniel Veillard
On Thu, Oct 16, 2008 at 12:34:05PM -0700, Ian Main wrote:
 On Mon, 13 Oct 2008 15:56:21 +0200
 Daniel Veillard [EMAIL PROTECTED] wrote:
 
  On Wed, Sep 24, 2008 at 10:55:36AM -0700, Ian Main wrote:
I'm a little bit worried by the following:
  
PID USER  PR  NI  VIRT  RES  SHR S %CPU %MEMTIME+  COMMAND
   9758 qpidd 20   0  273m 137m 1644 S 46.6  6.8 986:30.29 qpidd
  
   that's with just 3 nodes over a week with no traffic except the
  keeping the machines on the bus, the memory footprint of qpidd
  seems to be growing, very slowly but nonetheless continuously, and
  that's just with 3 nodes hooked with libvirt-qpid.
   is that a genuine leak in qpidd ? (qpidd-0.3.700546-1.fc9.x86_64)
   is that related to the policy to keep messages sent to the QPid bus ?
  
I wonder. It also consumes some non-neglective amount of CPU as
  a background task.
 
 I talked to the qpid folks about this today.  It's a bug one way or another.
 They fixed one that may be causing this and there will be new RPMs out 
 hopefully today.  If that doesn't do it it may have something to do with
 how QMF is doing things.  If you still see this after the RPM upgrade
 let me know.  I'll be trying it out too.  Thanks!

  Okay, I will keep an eye, thanks for the info !

Daniel

-- 
Daniel Veillard  | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
[EMAIL PROTECTED]  | 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