Re: [Qemu-devel] [PULL 08/22] virtio-pci: Proxy for virtio-pmem

[Pankaj Gupta]

Hi Peter,

> > From: Pankaj Gupta <address@hidden>
> >
> > We need a proxy device for virtio-pmem, and this device has to be the
> > actual memory device so we can cleanly hotplug it.
> >
> > Forward memory device class functions either to the actual device or use
> > properties of the virtio-pmem device to implement these in the proxy.
> >
> > virtio-pmem will only be compiled for selected, supported architectures
> > (that can deal with virtio/pci devices being memory devices). An
> > architecture that is prepared for that can simply enable
> > CONFIG_VIRTIO_PMEM to make it work.
> >
> > As not all architectures support memory devices (and CONFIG_VIRTIO_PMEM
> > will be enabled per supported architecture), we have to move the PCI proxy
> > to a separate file.
> >
> > Signed-off-by: Pankaj Gupta <address@hidden>
> > [ split up patches, memory-device changes, move pci proxy]
> > Signed-off-by: David Hildenbrand <address@hidden>
> > Message-Id: <address@hidden>
> > Reviewed-by: Michael S. Tsirkin <address@hidden>
> > Signed-off-by: Michael S. Tsirkin <address@hidden>
> > ---
> 
> Hi; Coverity spotted a bug here (CID 1403010):
> 
> > +static uint64_t virtio_pmem_pci_get_plugged_size(const MemoryDeviceState
> > *md,
> > +                                                 Error **errp)
> > +{
> > +    VirtIOPMEMPCI *pci_pmem = VIRTIO_PMEM_PCI(md);
> > +    VirtIOPMEM *pmem = VIRTIO_PMEM(&pci_pmem->vdev);
> > +    VirtIOPMEMClass *vpc = VIRTIO_PMEM_GET_CLASS(pmem);
> > +    MemoryRegion *mr = vpc->get_memory_region(pmem, errp);
> > +
> > +    /* the plugged size corresponds to the region size */
> > +    return mr ? 0 : memory_region_size(mr);
> 
> This looks like maybe the arguments to ?: have been put
> the wrong way round? If mr is non-NULL we'll return 0
> and if it is NULL then we'll crash because memory_region_size()
> dereferences mr...

Yes. I will send a patch to fix this.

Thanks,
Pankaj

> 
> > +}
> 
> thanks
> -- PMM
> 
>