On 05/11/2013 11:33 AM, Cole Robinson wrote:
> 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()
>
Also I just pushed a commit to tests/testdriver.xml to add duplicate devices,
and a guest named test-duplicate-usb so you can this logic without having to
interact with real guests/hardware.
- Cole
_______________________________________________
virt-tools-list mailing list
virt-tools-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/virt-tools-list
