Re: Re: [PATCH 13/33] qemu: Drop qemuDomainSetSCSIControllerModel()
On Thu, Jan 25, 2024 at 10:16:55 -0800, Andrea Bolognani wrote:
On Thu, Jan 25, 2024 at 11:17:46AM +0100, Peter Krempa wrote:
> On Thu, Jan 25, 2024 at 11:15:14 +0100, Peter Krempa wrote:
> > On Wed, Jan 24, 2024 at 20:37:33 +0100, Andrea Bolognani wrote:
> > > @@ -5584,7 +5584,9 @@
qemuDomainControllerDefPostParse(virDomainControllerDef *cont,
> > > switch (cont->type) {
> > > case VIR_DOMAIN_CONTROLLER_TYPE_SCSI:
> > > /* Set the default SCSI controller model if not already set */
> > > - if (qemuDomainSetSCSIControllerModel(def, cont, qemuCaps) <
0)
> > > + cont->model = qemuDomainGetSCSIControllerModel(def, cont,
qemuCaps);
> > > +
> > > + if (cont->model < 0)
> > > return -1;
> >
> > This is not future proof for the case when 'model' gets turned into a
> > proper enum.
>
> Based on further patches, it seems to be a problem in more places.
I'm not sure I understand. Can you please explain what "a proper
enum" means in this context? Is that connected to some condition that
If you turn the 'model' member from 'int' to the appropriate enum type
the '< 0' check will no longer work if the enum has no negative members.
This is due to the fact that the compiler then treats the enum as
'unsigned'
could present itself in the current code, or as a consequence of a
potential future refactoring? Based on the fact that you ultimately
ACKed the patch, I assume the latter.
There are multiple other instances of exactly the same problem for this
enum and additionally this one has a native -1 member, so my point was
moot mostly.