[virt-tools-list] [PATCH v2] domain: add 'pre-startup' signal and do nodedevs checking

[Cole Robinson]

On 05/10/2013 05:49 AM, Guannan Ren wrote:
> This patch introduces 'pre-start' signal and registers
> nodedev checking handler to check duplicate USB devices.
> If virt-manager can not identify unique usb device any more
> before domain startup, it will throw a tip error to tell it
> is time to reattach host USB devices to get updated bus/addr info.
> ---
>  virtManager/domain.py | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/virtManager/domain.py b/virtManager/domain.py
> index 791f2af..26cff37 100644
> --- a/virtManager/domain.py
> +++ b/virtManager/domain.py
> @@ -149,6 +149,7 @@ class vmmDomain(vmmLibvirtObject):
>          "status-changed": (GObject.SignalFlags.RUN_FIRST, None, [int, int]),
>          "resources-sampled": (GObject.SignalFlags.RUN_FIRST, None, []),
>          "inspection-changed": (GObject.SignalFlags.RUN_FIRST, None, []),
> +        "pre-startup": (GObject.SignalFlags.RUN_FIRST, None, [])
>      }
>  
>      def __init__(self, conn, backend, uuid):
> @@ -194,6 +195,9 @@ class vmmDomain(vmmLibvirtObject):
>          self._stats_disk_supported = True
>          self._stats_disk_skip = []
>  
> +        # pre_startup global flags
> +        self._prestartup_hostdev_check_failed = False
> +
>          self.inspection = vmmInspectionData()
>  
>          if isinstance(self._backend, virtinst.Guest):
> @@ -252,7 +256,59 @@ class vmmDomain(vmmLibvirtObject):
>  
>          self.connect("status-changed", self._update_start_vcpus)
>          self.connect("config-changed", self._reparse_xml)
> +        self.connect("pre-startup", self._prestartup_nodedev_check)
> +
> +    #####################
> +    # checking routines #
> +    #####################
> +
> +    def attrVal(self, attr):
> +        if hasattr(self, attr):
> +            return getattr(self, attr)
> +        return False
> +
> +    # class closure to initialize global variable.
> +    def do_pre_startup(self):
> +        if getattr(self, "_prestartup_hostdev_check_failed"):
> +            self._prestartup_hostdev_check_failed = False
> +            # Initialize falure reasons
> +            self._hostdev_non_exist = False
> +            self._hostdev_multiples = False
>  
> +    def _prestartup_nodedev_check(self, data=None):
> +        for hostdev in self.get_hostdev_devices():
> +            devtype = hostdev.type
> +
> +            if devtype != "usb":
> +                continue
> +
> +            vendor = hostdev.vendor
> +            product = hostdev.product
> +            bus = hostdev.bus
> +            device = hostdev.device
> +
> +            if vendor and product:
> +                count = self.conn.get_nodedevs_number("usb_device",
> +                                                      vendor,
> +                                                      product)
> +                if not count:
> +                    self._prestartup_hostdev_check_failed = True
> +                    self._hostdev_non_exist = True
> +
> +                elif count > 1 and not (bus and device):
> +                    self._prestartup_hostdev_check_failed = True
> +                    self._hostdev_multiples = True
> +
> +    def _prestartup_nodedev_report(self):
> +        if self._prestartup_hostdev_check_failed:
> +            if self.attrVal("_hostdev_non_exist"):
> +                raise RuntimeError(_("Could not find any USB device"))
> +

I would drop this error and just let libvirt/qemu handle it for us.

> +            elif self.attrVal("_hostdev_multiples"):
> +                raise RuntimeError(_("The attached USB device "
> +                                     "is not unique, please remove "
> +                                     "and add it again!"))

I'd change this message to:

There is more than one '%(prettyname)s' device attached to your host, and we
can't determine which one to use for your guest.

To fix this, remove and reattach the USB device to your guest using the 'Add
Hardware' wizard.

> +        return;
>  
>      ###########################
>      # Misc API getter methods #
> @@ -1171,6 +1227,10 @@ class vmmDomain(vmmLibvirtObject):
>          if self.get_cloning():
>              raise RuntimeError(_("Cannot start guest while cloning "
>                                   "operation in progress"))
> +
> +        self.emit("pre-startup")
> +        self._prestartup_nodedev_report()
> +
>          self._backend.create()
>          self.idle_add(self.force_update_status)
>  
> 

Hmm, that return value mangling is sub-optimal, but indeed I can't get emit to
return values like I thought it could.

But I remembered a trick we use elsewhere in virt-manager already. If you pass
a list as a signal argument, the signal handler can add values to it, which
the emitter can than access. You can then just have the signal callback return
the string error directly.

Here's an example to show how it works:


diff --git a/virtManager/domain.py b/virtManager/domain.py
index 7c3f952..f2a9041 100644
--- a/virtManager/domain.py
+++ b/virtManager/domain.py
@@ -149,7 +149,7 @@ class vmmDomain(vmmLibvirtObject):
         "status-changed": (GObject.SignalFlags.RUN_FIRST, None, [int, int]),
         "resources-sampled": (GObject.SignalFlags.RUN_FIRST, None, []),
         "inspection-changed": (GObject.SignalFlags.RUN_FIRST, None, []),
-        "pre-startup": (GObject.SignalFlags.RUN_FIRST, None, [])
+        "pre-startup": (GObject.SignalFlags.RUN_FIRST, None, [object])
     }

     def __init__(self, conn, backend, uuid):
@@ -275,7 +275,10 @@ class vmmDomain(vmmLibvirtObject):
             self._hostdev_non_exist = False
             self._hostdev_multiples = False

-    def _prestartup_nodedev_check(self, data=None):
+    def _prestartup_nodedev_check(self, src, ret):
+        print "in hook ret", ret
+        ret.append("frobber")
+        return
         for hostdev in self.get_hostdev_devices():
             devtype = hostdev.type

@@ -1228,7 +1231,9 @@ class vmmDomain(vmmLibvirtObject):
             raise RuntimeError(_("Cannot start guest while cloning "
                                  "operation in progress"))

-        self.emit("pre-startup")
+        ret = []
+        self.emit("pre-startup", ret)
+        print "emit ret", ret
         self._prestartup_nodedev_report()

         self._backend.create()