[libvirt] [PATCH] Clarify lack of generated IDE -device string in QEMU driver

[Matthew Booth]

On 28/01/10 10:57, Daniel P. Berrange wrote:
> On Thu, Jan 28, 2010 at 10:35:48AM +0000, Matthew Booth wrote:
>> The QEMU driver contained code to generate a -device string for piix4-ide, but
>> wasn't using it. This was intentional. This change removes the string generation
>> and adds a comment explaining why no -device is necessary.
>>
>> * src/qemu/qemu_conf.c: Remove VIR_DOMAIN_CONTROLLER_TYPE_IDE handler in
>>                         qemuBuildControllerDevStr(). Add comments.
>> ---
>>  src/qemu/qemu_conf.c |   16 ++++++++--------
>>  1 files changed, 8 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
>> index f4a6c08..1c4f326 100644
>> --- a/src/qemu/qemu_conf.c
>> +++ b/src/qemu/qemu_conf.c
>> @@ -2121,11 +2121,8 @@ qemuBuildControllerDevStr(virDomainControllerDefPtr def)
>>          virBufferVSprintf(&buf, ",id=scsi%d", def->idx);
>>          break;
>>  
>> +    /* We always get an IDE controller, whether we want it or not. */
>>      case VIR_DOMAIN_CONTROLLER_TYPE_IDE:
>> -        virBufferAddLit(&buf, "piix4-ide");
>> -        virBufferVSprintf(&buf, ",id=ide%d", def->idx);
>> -        break;
>> -
>>      default:
>>          goto error;
>>      }
>> @@ -3141,16 +3138,19 @@ int qemudBuildCommandLine(virConnectPtr conn,
>>  
>>      if (qemuCmdFlags & QEMUD_CMD_FLAG_DEVICE) {
>>          for (i = 0 ; i < def->ncontrollers ; i++) {
>> -            char *scsi;
>> -            if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI)
>> +            /* We don't add an explicit IDE controller because the provided
>> +             * PIIX4 device already includes one. It isn't possible to remove
>> +             * the PIIX4. */
>> +            if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_IDE)
>>                  continue;
> 
> That's not quite right because you changed the conditional to blacklist IDE
> insteasd of whitelisting SCSI. This means that FDC now falls through. We do
> not need to explicitly add the FDC, since that is also provided automatically
> on the ISA bridge that's behind the PIIX4.

Ok. Still think a blacklist is more appropriate, though. I'll add FDC to
the blacklist. Are there any more?

Matt
-- 
Matthew Booth, RHCA, RHCSS
Red Hat Engineering, Virtualisation Team

M:       +44 (0)7977 267231
GPG ID:  D33C3490
GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490