On 01/05/2015 02:29 AM, Wang Rui wrote:
Signed-off-by: Wang Rui <moon.wangrui@huawei.com> Signed-off-by: Zhou Yimin <zhouyimin@huawei.com> --- src/conf/domain_conf.c | 43 +++++++++++++++++++++++-------------------- 1 file changed, 23 insertions(+), 20 deletions(-)
Since there's no commit message - even more reason to combine with patch 3 (or my 3b)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index d2c4a0a..5beb830 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -19991,29 +19991,32 @@ virDomainDefCompatibleDevice(virDomainDefPtr def, { virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev);
- if (action != VIR_DOMAIN_DEVICE_ACTION_ATTACH) - return 0; - - if (!virDomainDefHasUSB(def) && - STRNEQ(def->os.type, "exe") && - virDomainDeviceIsUSB(dev)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Device configuration is not compatible: " - "Domain has no USB bus support")); - return -1; - } - - if (info && info->bootIndex > 0) { - if (def->os.nBootDevs > 0) { + switch (action) { + case VIR_DOMAIN_DEVICE_ACTION_ATTACH: + if (!virDomainDefHasUSB(def) && + STRNEQ(def->os.type, "exe") && + virDomainDeviceIsUSB(dev)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("per-device boot elements cannot be used" - " together with os/boot elements")); + _("Device configuration is not compatible: " + "Domain has no USB bus support")); return -1; } - if (virDomainDeviceInfoIterate(def, - virDomainDeviceInfoCheckBootIndex, - info) < 0) - return -1; + case VIR_DOMAIN_DEVICE_ACTION_UPDATE: + if (info && info->bootIndex > 0) { + if (def->os.nBootDevs > 0) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("per-device boot elements cannot be used" + " together with os/boot elements")); + return -1; + } + if (virDomainDeviceInfoIterate(def, + virDomainDeviceInfoCheckBootIndex, + info) < 0) + return -1; + } + break; + default: + return 0;
The 'norm' of using default is to list all the cases - makes it easier to determine when one is missing. I think the switch in this case is overkill. I think this is better: if (action != VIR_DOMAIN_DEVICE_ACTION_ATTACH || action != VIR_DOMAIN_DEVICE_ACTION_UPDATE) return 0; if (action == VIR_DOMAIN_DEVICE_ACTION_ATTACH && !virDomainDefHasUSB(def) && STRNEQ(def->os.type, "exe") && virDomainDeviceIsUSB(dev)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("Device configuration is not compatible: " "Domain has no USB bus support")); return -1; } if (info && info->bootIndex > 0) { if (def->os.nBootDevs > 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("per-device boot elements cannot be used" " together with os/boot elements")); return -1; } if (virDomainDeviceInfoIterate(def, virDomainDeviceInfoCheckBootIndex, info) < 0) return -1; } [sorry about the wrapping on the CheckBootIndex call - the point is all you're doing is adding _UPDATE to the initial check... then adding the "if (action == *_ATTACH)" for just the if USB check...
}
return 0;