On 30.09.2015 16:38, Jiri Denemark wrote:
> On Fri, Sep 18, 2015 at 18:05:47 +0300, Nikolay Shirokovskiy wrote:
>> Move virDomainMigrateUnmanagedProto* expected params list check into
>> function itself and use common virTypedParamsCheck for this purpose.
>>
>> Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@xxxxxxxxxxxxx>
>> ---
>> src/libvirt-domain.c | 56 ++++++++++++++++++++-----------------------------
>> 1 files changed, 23 insertions(+), 33 deletions(-)
>>
>> diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
>> index f87a22d..abed9d6 100644
>> --- a/src/libvirt-domain.c
>> +++ b/src/libvirt-domain.c
>> @@ -3322,30 +3322,31 @@ virDomainMigrateUnmanagedProto2(virDomainPtr domain,
>> int nparams,
>> unsigned int flags)
>> {
>> + /* uri parameter is added for direct case */
>> + const char *compatParams[] = { VIR_MIGRATE_PARAM_DEST_NAME,
>> + VIR_MIGRATE_PARAM_BANDWIDTH,
>> + VIR_MIGRATE_PARAM_URI };
>> const char *uri = NULL;
>> const char *miguri = NULL;
>> const char *dname = NULL;
>> - const char *xmlin = NULL;
>> unsigned long long bandwidth = 0;
>>
>> + if (!virTypedParamsCheck(params, nparams, compatParams,
>> + ARRAY_CARDINALITY(compatParams))) {
>> + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>> + _("Migration does not support some of parameters."));
>
> How about "Some parameters are not supported by migration protocol 2"?
>
>> + return -1;
>> + }
>> +
>> if (virTypedParamsGetString(params, nparams,
>> VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
>> virTypedParamsGetString(params, nparams,
>> VIR_MIGRATE_PARAM_DEST_NAME, &dname) < 0 ||
>> - virTypedParamsGetString(params, nparams,
>> - VIR_MIGRATE_PARAM_DEST_XML, &xmlin) < 0 ||
>> virTypedParamsGetULLong(params, nparams,
>> VIR_MIGRATE_PARAM_BANDWIDTH, &bandwidth) < 0) {
>> return -1;
>> }
>>
>> - if (xmlin) {
>> - virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>> - _("Unable to change target guest XML during "
>> - "migration"));
>> - return -1;
>> - }
>> -
>> if (flags & VIR_MIGRATE_PEER2PEER) {
>> if (miguri) {
>> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> _("Unable to override peer2peer migration URI"));
> return -1;
> }
>
> Can we merge this check with with the others by adding
> VIR_MIGRATE_PARAM_URI to compatParams only if VIR_MIGRATE_PEER2PEER was
> passed? I think it would be a bit cleaner, although it depends on how
> dirty way of doing that you choose :-)
I'm not sure is this considered dirty?
1. char** cast (could be fixed by changing function signature to const char * const *
2. unchecked array accessing - safe until somebody will edit this code
const char *compatParams[3] = { VIR_MIGRATE_PARAM_DEST_NAME,
VIR_MIGRATE_PARAM_BANDWIDTH,
NULL};
size_t ncompatParams = 0;
ncompatParams = virStringListLength((char**)compatParams);
if (!(flags & VIR_MIGRATE_PEER2PEER))
compatParams[ncompatParams++] = VIR_MIGRATE_PARAM_URI;
>
>> @@ -3369,11 +3370,22 @@ virDomainMigrateUnmanagedProto3(virDomainPtr domain,
>> int nparams,
>> unsigned int flags)
>> {
>> + const char *compatParams[] = { VIR_MIGRATE_PARAM_URI,
>> + VIR_MIGRATE_PARAM_DEST_NAME,
>> + VIR_MIGRATE_PARAM_DEST_XML,
>> + VIR_MIGRATE_PARAM_BANDWIDTH };
>> const char *miguri = NULL;
>> const char *dname = NULL;
>> const char *xmlin = NULL;
>> unsigned long long bandwidth = 0;
>>
>> + if (!virTypedParamsCheck(params, nparams, compatParams,
>> + ARRAY_CARDINALITY(compatParams))) {
>> + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
>> + _("Migration does not support some of parameters."));
>
> How about "Some parameters are not supported by migration protocol 3"?
>
>> + return -1;
>> + }
>> +
>> if (virTypedParamsGetString(params, nparams,
>> VIR_MIGRATE_PARAM_URI, &miguri) < 0 ||
>> virTypedParamsGetString(params, nparams,
>
> Jirka
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list