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

Re: [libvirt] [PATCHv2 12/62] qemu: monitor: Allow using 'qdev' instead of 'device' for getting disk throttling



On Wed, Aug 22, 2018 at 06:47:45 -0400, John Ferlan wrote:
> [...]
> 
> > --- a/src/qemu/qemu_monitor_json.c
> > +++ b/src/qemu/qemu_monitor_json.c
> > @@ -4818,7 +4818,8 @@ int qemuMonitorJSONOpenGraphics(qemuMonitorPtr mon,
> >      }
> >  static int
> >  qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle,
> > -                                   const char *device,
> > +                                   const char *drivealias,
> > +                                   const char *qdevid,
> >                                     virDomainBlockIoTuneInfoPtr reply)
> >  {
> >      int ret = -1;
> > @@ -4828,7 +4829,8 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle,
> >      for (i = 0; i < virJSONValueArraySize(io_throttle); i++) {
> >          virJSONValuePtr temp_dev = virJSONValueArrayGet(io_throttle, i);
> >          virJSONValuePtr inserted;
> > -        const char *current_dev;
> > +        const char *current_drive;
> > +        const char *current_qdev;
> > 
> >          if (!temp_dev || virJSONValueGetType(temp_dev) != VIR_JSON_TYPE_OBJECT) {
> >              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > @@ -4837,14 +4839,18 @@ qemuMonitorJSONBlockIoThrottleInfo(virJSONValuePtr io_throttle,
> >              goto cleanup;
> >          }
> > 
> > -        if (!(current_dev = virJSONValueObjectGetString(temp_dev, "device"))) {
> > +        current_qdev = virJSONValueObjectGetString(temp_dev, "qdev");
> > +        current_drive = virJSONValueObjectGetString(temp_dev, "device");
> > +
> > +        if (!current_drive && !current_qdev) {
> 
> Is this supposed to be || ?

No that would break old qemus

> 
> >              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> >                             _("block_io_throttle device entry "
> >                               "was not in expected format"));
> >              goto cleanup;
> >          }
> > 
> > -        if (STRNEQ(current_dev, device))
> > +        if ((drivealias && STRNEQ(current_drive, drivealias)) ||
> > +            (qdevid && STRNEQ(current_qdev, qdevid)))
> 
> Because Coverity complains here that if one or the other is == NULL
> while the other != NULL, then strcmp is not going to be happy.

Ah, yes. I'll change these to the NULLABLE variant.

Attachment: signature.asc
Description: PGP signature


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