[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH v3 6/9] qemu_command: Honour chardev alias assignment with a function



On 03.07.2013 17:29, Daniel P. Berrange wrote:
> On Tue, Jul 02, 2013 at 05:53:05PM +0200, Michal Privoznik wrote:
>> The chardev alias assignment is going to be needed in a separate
>> places, so it should be moved into a separate function rather
>> than copying code randomly around.
>> ---
>>  src/qemu/qemu_command.c | 75 +++++++++++++++++++++++++++++++++++++++++++------
>>  src/qemu/qemu_command.h |  3 ++
>>  2 files changed, 70 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>> index ba93233..903839f 100644
>> --- a/src/qemu/qemu_command.c
>> +++ b/src/qemu/qemu_command.c
>> @@ -892,6 +892,65 @@ qemuAssignDeviceControllerAlias(virDomainControllerDefPtr controller)
>>      return 0;
>>  }
>>  
>> +int
>> +qemuAssignDeviceChrAlias(virDomainDefPtr def,
>> +                         virDomainChrDefPtr chr,
>> +                         ssize_t idx)
>> +{
>> +    const char *prefix = NULL;
>> +    const char *prefix2 = NULL;
>> +
>> +    switch ((enum virDomainChrDeviceType) chr->deviceType) {
>> +    case VIR_DOMAIN_CHR_DEVICE_TYPE_PARALLEL:
>> +        prefix = "parallel";
>> +        break;
>> +
>> +    case VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL:
>> +        prefix = "serial";
>> +        break;
>> +
>> +    case VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE:
>> +        prefix = "console";
>> +        prefix2 = "serial";
>> +        break;
>> +
>> +    case VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL:
>> +        prefix = "channel";
>> +        break;
>> +
>> +    case VIR_DOMAIN_CHR_DEVICE_TYPE_LAST:
>> +        return -1;
>> +    }
>> +
>> +    if (idx == -1) {
>> +        virDomainChrDefPtr **arrPtr;
>> +        size_t *cntPtr;
>> +        size_t i;
>> +        idx = 0;
>> +
>> +        virDomainChrGetDomainPtrs(def, chr, &arrPtr, &cntPtr);
>> +
>> +        for (i = 0; i < *cntPtr; i++) {
>> +            int thisidx;
>> +            if (((thisidx = qemuDomainDeviceAliasIndex(&(*arrPtr)[i]->info, prefix)) < 0) &&
>> +                (prefix2 &&
>> +                 (thisidx = qemuDomainDeviceAliasIndex(&(*arrPtr)[i]->info, prefix2)) < 0)) {
>> +                virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
>> +                               _("Unable to determine device index for character device"));
>> +                return -1;
>> +            }
>> +            if (thisidx >= idx)
>> +                idx = thisidx + 1;
>> +        }
>> +    }
> 
> The commit message describes this as a simple refactoring, but this
> if (idx== -1) {...}  is all new functionality compared to what is
> being replaced. I'm not too sure that this logic is correct
> either when dealing with <console> with a 'serialXX' alias.

This function just imitates other functions we've already:
qemuAssignDeviceRedirdevAlias, qemuAssignDeviceHostdevAlias being
examples. One can find even more.

And regarding <console> I've sent 2 patches, none of them was accepted.
So I had to workaround the problem in my patch. What's the solution
you're suggesting? Just to refresh our memory: for <console> the alias
can be either 'consoleX' or 'serialX' depending if the daemon was
restarted or not.

Michal


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]