Re: [libvirt] [PATCH 6/8] conf: add compatiblity check for boot index when updating device
On 01/05/2015 02:29 AM, Wang Rui wrote:
Signed-off-by: Wang Rui <moon.wangrui(a)huawei.com>
Signed-off-by: Zhou Yimin <zhouyimin(a)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;