On 01/05/2015 02:29 AM, Wang Rui wrote:
> Signed-off-by: Wang Rui <moon.wangrui@xxxxxxxxxx>
> Signed-off-by: Zhou Yimin <zhouyimin@xxxxxxxxxx>
> ---
> 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;
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list