[et-mgmt-tools] [PATCH] Check the making domain's disk sources

[Tatsuro Enokura]

Hi, Dan

Daniel P. Berrange wrote:
>> The virt-install command specify the Domain's disk sources.
>> But the virt-install command doesn't check that the disk sources are
>> conflicted among the other domains.
>>
>> The attached patch resolve this issue in the following way:
>>
>>   1) Count that the making domian's disk src is already use
>>      the exists domain's disk source.
>>   2) if count > 0, print confirmation message.
>>   3) if type yes then leave disk source as it is,
>>      else ask disk source again.
>>
>> I am making a patch for virt-manager about this issue.
> 
> If you the non-interactive bit of code into the virtinst/Guest.py module
> in the VirtualDisk class, we can share 95% of it between both virtinstall 
> & virt-manager.

Thank you for your suggestion.
I rewrite the patch.

Signed-off-by: Tatsuro Enokura <fj7716hz at aa.jp.fujitsu.com>

Thanks,
Tatsuro Enokura

-----------------------------------------------------------------------------
diff -r ecc4386895aa virt-install
--- a/virt-install      Thu Mar 15 12:09:51 2007 -0400
+++ b/virt-install      Fri Mar 16 11:32:30 2007 +0900
@@ -103,7 +103,7 @@ def get_keymap(keymap, guest):
         except ValueError, e:
             print "ERROR: ", e

-def get_disk(disk, size, sparse, guest, hvm):
+def get_disk(disk, size, sparse, guest, hvm, conn):
     # FIXME: need to handle a list of disks at some point
     while 1:
         msg = "What would you like to use as the disk (path)?"
@@ -123,6 +123,23 @@ def get_disk(disk, size, sparse, guest,

         try:
             d = virtinst.VirtualDisk(disk, size, sparse = sparse)
+            if d.is_conflict_disk(conn) is True:
+                while 1:
+                    retryFlg = False
+                    warnmsg = "Disk %s is already in use by another
guest!" % disk
+                    res = prompt_for_input(warnmsg + "  Do you really
want to use the disk (yes or no)? ")
+                    try:
+                        if yes_or_no(res) is True:
+                            break
+                        else:
+                            retryFlg = True
+                            break
+                    except ValueError, e:
+                        print "ERROR: ", e
+                        continue
+                if retryFlg is True:
+                    disk = size = None
+                    continue
             if d.type == virtinst.VirtualDisk.TYPE_FILE and not(hvm)
and virtinst.util.is_blktap_capable():
                 d.driver_name = virtinst.VirtualDisk.DRIVER_TAP
         except ValueError, e:
@@ -133,7 +150,7 @@ def get_disk(disk, size, sparse, guest,
         guest.disks.append(d)
         break

-def get_disks(disk, size, sparse, guest, hvm):
+def get_disks(disk, size, sparse, guest, hvm, conn):
     # ensure we have equal length lists
     if (type(disk) == type(size) == list):
         if len(disk) != len(size):
@@ -145,13 +162,13 @@ def get_disks(disk, size, sparse, guest,
         disk = [ None ] * len(size)

     if (type(disk) == list):
-        map(lambda d, s: get_disk(d, s, sparse, guest, hvm),
+        map(lambda d, s: get_disk(d, s, sparse, guest, hvm, conn),
             disk, size)
     elif (type(size) == list):
-        map(lambda d, s: get_disk(d, s, sparse, guest, hvm),
+        map(lambda d, s: get_disk(d, s, sparse, guest, hvm, conn),
             disk, size)
     else:
-        get_disk(disk, size, sparse, guest, hvm)
+        get_disk(disk, size, sparse, guest, hvm, conn)

 def get_network(mac, bridge, guest):
     if mac == "RANDOM":
@@ -461,7 +478,7 @@ def main():

     # set up disks
     get_disks(options.diskfile, options.disksize, options.sparse,
-              guest, hvm)
+              guest, hvm, conn)

     # set up network information
     get_networks(options.mac, options.bridge, guest)
diff -r ecc4386895aa virtinst/Guest.py
--- a/virtinst/Guest.py Thu Mar 15 12:09:51 2007 -0400
+++ b/virtinst/Guest.py Fri Mar 16 11:36:50 2007 +0900
@@ -132,6 +132,42 @@ class VirtualDisk:
             ret += "      <readonly/>\n"
         ret += "    </disk>\n"
         return ret
+
+    def is_conflict_disk(self, conn):
+        vms = []
+        # get working domain's name
+        ids = conn.listDomainsID();
+        for id in ids:
+            vm = conn.lookupByID(id)
+            vms.append(vm)
+        # get defined domain
+        names = conn.listDefinedDomains()
+        for name in names:
+            vm = conn.lookupByName(name)
+            vms.append(vm)
+
+        count = 0
+        for vm in vms:
+            doc = None
+            try:
+                doc = libxml2.parseDoc(vm.XMLDesc(0))
+            except:
+                continue
+            ctx = doc.xpathNewContext()
+            try:
+                try:
+                    count +=
ctx.xpathEval("count(/domain/devices/disk/source[@dev='%s'])" % self.path)
+                except:
+                    continue
+            finally:
+                if ctx is not None:
+                    ctx.xpathFreeContext()
+                if doc is not None:
+                    doc.freeDoc()
+        if count > 0:
+            return True
+        else:
+            return False

     def __repr__(self):
         return "%s:%s" %(self.type, self.path)
-----------------------------------------------------------------------------