Re: [libvirt] [PATCH v5 02/16] qemu: Introduce qemuDomainDeviceDefValidateControllerAttributes

On 01/29/2018 12:53 PM, Ján Tomko wrote: As usual a seemingly simple request sends me down the rabbit hole. Setting a default controller model in qemuDomainControllerDefPostParse would require passing @qemuCaps - simple enough until one checks out the caller qemuDomainDeviceDefPostParse which notes: " /* Note that qemuCaps may be NULL when this function is called. This * function shall not fail in that case. It will be re-run on VM startup * with the capabilities populated. */ " <sigh> So, would adding the following to patch 1 for the Reset function suffice? /** qemuDomainResetSCSIControllerModel * @def: Domain def * @qemuCaps: QEMU capabilities * @model: model to reset "default" to - either by @def or @qemuCaps * * Typically qemuDomainControllerDefPostParse would be used to set * a default controller model; however, qemuDomainDeviceDefPostParse * can be passed a NULL @qemuCaps, so setting the default model may * not be possible. Thus we're left to calling this Reset function * from numerous locations that need to get the default model for * the controller when one is not defined. * * Returns 0 on success, -1 on failure */ I can also add a note in qemuDomainDeviceDefValidateController that restates this for this patch, e.g.: if (controller->type == VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { /* Resetting the default should have been handled during post parse * parse callback; however, at that time we could not guarantee that * qemuCaps was valid, so we're left doing this now */ if ((qemuDomainResetSCSIControllerModel(def, qemuCaps, &model)) < 0) return -1; } virtio-scsi was never the default AFAIK... That wasn't the point/problem. The problem is described in the next paragraph. I looked it up, this is handled by c52dbafe and 07beea6c John