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

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.