Re: [PATCH] Fix restore handling checks

[Michal Novotny <minovotn@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3186 bytes --]

On 06/22/2010 07:43 AM, Michal Novotny wrote:
> On 06/21/2010 08:04 PM, Keir Fraser wrote:
>> On 21/06/2010 17:30, "Michal Novotny"<minovotn@redhat.com>  wrote:
>>
>>> Hi,
>>> this is the patch to fix restore handling to implement some more checks
>>> to support more checks than for UUID and name duplicity. This patch
>>> basically disallows the migration/restore of IDE drives with the
>>> read-only flag since this is not supported according to the ATAPI/IDE
>>> specifications so we should disallow this for both domain creation and
>>> domain migration/restore.
>> What about CD-ROMs? This would break my test domain config, for example.
>>
>
> Right, there's the exception for CD-ROMs according to the spec. I 
> should implement this as well but read-only IDE disk devices are not 
> supported according to the IDE specs.


This is the updated version of my patch to allow read-only CD-ROM 
devices as you had a good point that read-only is supported for CD-ROM 
IDE drives (but only for them).


>
>>> This patch implements it for both create and
>>> restore/migrate functionality.
>>>
>>> Also, the check whether the host machine does have enough memory
>>> available for the guest has been implemented which can be the real 
>>> issue
>>> when you try to migrate a guest from one machine to another that is not
>>> having enough memory for this guest. The guest memory gets transferred
>>> but it fails to run so it's not running on either of those machines
>>> (i.e. domain is not on the destination nor source host machine).
>> Failed restore should get reported back to the host that is saving 
>> the guest
>> state, and cause that machine to resume execution of the original VM. 
>> Does
>> that not work for you?
>>
>> Possibly checking up front for available memory on the target is a good
>> idea, but it shouldn't be *essential* if the error handling is up to 
>> par.
>>
>>   -- Keir
>

I was able to make it working now and this functionality seems to be 
working now however I think that the preliminary check before the 
transfer itself is a good idea. As far as I had it implemented already I 
just did some modifications to allow CD-ROM IDE read-only drives and 
this is the updated version.

Michal

>
>>> I did try it with restore functionality now since I've been able to 
>>> make
>>> it working for save once so I'm currently using one save image for the
>>> testing but unfortunately I'm having many issues with the common
>>> migration and save functionality since I've been able to make it 
>>> working
>>> once to save it correctly. Fortunately the restores for this one
>>> particular save image is working fine. I was also thinking about 2
>>> concurrent migrations to the guest and/or save with the concurrent
>>> migration and it should be the issue (although it's not been tested
>>> because of reasons described above) since the domain gets created and
>>> it's available in the XendDomain list (i.e. xc.domain_getinfo() 
>>> list) so
>>> it shouldn't be an issue here.
>>>
>>> Michal
>>>
>>> Signed-off-by: Michal Novotny<minovotn@redhat.com>
>>
>
>


-- 
Michal Novotny<minovotn@redhat.com>, RHCE
Virtualization Team (xen userspace), Red Hat


[-- Attachment #2: xen-fix-restore-handling-for-ide-drives-and-memory.patch --]
[-- Type: text/x-patch, Size: 5129 bytes --]

diff -r 72c6228b5f0f tools/python/xen/xend/XendCheckpoint.py
--- a/tools/python/xen/xend/XendCheckpoint.py	Mon Jun 21 19:19:25 2010 +0100
+++ b/tools/python/xen/xend/XendCheckpoint.py	Tue Jun 22 08:07:48 2010 +0200
@@ -64,6 +64,71 @@
               list.insert (i+1, value)
     return
 
+def get_avail_memory():
+    """Get available memory for new guest creation (in KiB, for restore)"""
+    from xen.xend import XendOptions
+
+    # First get total memory in KiB
+    xc = xen.lowlevel.xc.xc()
+    info = xc.domain_getinfo()
+    total_mem  = xc.physinfo()['total_memory']
+    del xc
+
+    # Count memory of all running guests in KiB
+    mem_used = 0L
+    for x in info:
+        if x['domid'] != 0:
+            mem_used += x['mem_kb']
+
+    # Get minimal memory for dom0 and convert to KiB
+    min_mem = XendOptions.instance().get_dom0_min_mem() * 1024
+
+    return total_mem - mem_used - min_mem
+
+def check_for_restore_bail(msg):
+    raise VmError("Cannot restore: %s" % msg)
+
+def check_for_enough_mem(val):
+    mem_avail = int(get_avail_memory() / 1024)
+    log.debug("Available memory: %s MiB, guest requires: %s MiB" % (mem_avail, val))
+    return val > mem_avail
+
+def check_for_restore_is_hvm(cfg):
+    for item in cfg:
+        if (type(item) == list):
+           if item[0] == 'image':
+               return type(item[1]) == list and item[1][0] == 'hvm'
+
+def check_for_restore_hvm_have_readonly_ide(cfg):
+    """Check the configuration for read-only IDE devices
+       Fail if such a device is found."""
+    disallow = None
+    if type(cfg) == list and cfg[0] in ('tap', 'vbd'):
+        for p in cfg:
+            if (type(p) != str):
+                if p[0] == 'dev':
+                    disallow = ((p[1].find('hd') >= 0) and
+                                (p[1].find('cdrom') == -1))
+                if p[0] == 'mode' and disallow:
+                    if p[1] == 'r':
+                        return True
+    return False
+
+def check_for_restore(cfg):
+    is_hvm = check_for_restore_is_hvm(cfg)
+    name = None
+    for item in cfg:
+        if (type(item) == list):
+            if item[0] == 'name':
+                name = item[1]
+            # Check for enough memory to create the guest
+            if item[0] == 'memory':
+                if not check_for_enough_mem(item[1]):
+                    check_for_restore_bail('Host machine doesn\'t have enough memory for %s' % name)
+            # We disable read-only IDE drives only for HVM guests
+            if item[0] == 'device' and is_hvm:
+                if check_for_restore_hvm_have_readonly_ide(item[1]):
+                    check_for_restore_bail('HVM domains cannot be using read-only IDE drives')
 
 def save(fd, dominfo, network, live, dst, checkpoint=False, node=-1):
     from xen.xend import XendDomain
@@ -220,6 +285,7 @@
             othervm = xd.domain_lookup_nr(domconfig["uuid"])
         if othervm is not None and othervm.domid is not None: 
             raise VmError("Domain '%s' already exists with ID '%d'" % (domconfig["name_label"], othervm.domid))
+        check_for_restore(vmconfig)
 
     if dominfo:
         dominfo.resume()
diff -r 72c6228b5f0f tools/python/xen/xend/XendDomainInfo.py
--- a/tools/python/xen/xend/XendDomainInfo.py	Mon Jun 21 19:19:25 2010 +0100
+++ b/tools/python/xen/xend/XendDomainInfo.py	Tue Jun 22 08:07:48 2010 +0200
@@ -82,6 +82,32 @@
 log = logging.getLogger("xend.XendDomainInfo")
 #log.setLevel(logging.TRACE)
 
+def cfg_is_hvm(cfg):
+    for item in cfg:
+        if (type(item) == list):
+           if item[0] == 'image':
+               return type(item[1]) == list and item[1][0] == 'hvm'
+
+def cfg_hvm_have_readonly_ide_disks(cfg):
+    """Check whether the configuration for read-only IDE disks since
+       they are not supported according to IDE specifications."""
+    if not cfg_is_hvm(cfg):
+        return False
+
+    disallow = None
+    for item in cfg:
+        if (type(item) == list):
+            if item[0] == 'device':
+                if type(item[1]) == list and item[1][0] in ('tap', 'vbd'):
+                    for p in item[1]:
+                        if (type(p) != str):
+                            if p[0] == 'dev':
+                              disallow = ((p[1].find('hd') >= 0) and
+                                          (p[1].find('cdrom') == -1))
+                        if p[0] == 'mode' and disallow:
+                            if p[1] == 'r':
+                                return True
+    return False
 
 def create(config):
     """Creates and start a VM using the supplied configuration. 
@@ -100,6 +126,10 @@
         othervm = XendDomain.instance().domain_lookup_nr(domconfig["uuid"])
     if othervm is not None and othervm.domid is not None:
         raise VmError("Domain '%s' already exists with ID '%d'" % (domconfig["name_label"], othervm.domid))
+
+    if cfg_hvm_have_readonly_ide_disks(config):
+        raise VmError("HVM domains cannot be using read-only IDE drives")
+
     log.debug("XendDomainInfo.create(%s)", scrub_password(config))
     vm = XendDomainInfo(domconfig)
     try:

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel