[PATCH] pcie_sriov: Remove g_new assertion

Akihiko Odaki posted 1 patch 1 year ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231123075630.12057-1-akihiko.odaki@daynix.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>
hw/pci/pcie_sriov.c | 1 -
1 file changed, 1 deletion(-)
[PATCH] pcie_sriov: Remove g_new assertion
Posted by Akihiko Odaki 1 year ago
g_new() aborts if the allocation fails so it returns NULL only if the
requested allocation size is zero. register_vfs() makes such an
allocation if NumVFs is zero so it should not assert that g_new()
returns a non-NULL value.

Fixes: 7c0fa8dff8 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)")
Buglink: https://issues.redhat.com/browse/RHEL-17209
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 hw/pci/pcie_sriov.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 5ef8950940..a1fe65f5d8 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -178,7 +178,6 @@ static void register_vfs(PCIDevice *dev)
     num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
 
     dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
-    assert(dev->exp.sriov_pf.vf);
 
     trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
                              PCI_FUNC(dev->devfn), num_vfs);
-- 
2.42.1
Re: [PATCH] pcie_sriov: Remove g_new assertion
Posted by Cédric Le Goater 1 year ago
On 11/23/23 08:56, Akihiko Odaki wrote:
> g_new() aborts if the allocation fails so it returns NULL only if the
> requested allocation size is zero. register_vfs() makes such an
> allocation if NumVFs is zero so it should not assert that g_new()
> returns a non-NULL value.
> 
> Fixes: 7c0fa8dff8 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)")
> Buglink: https://issues.redhat.com/browse/RHEL-17209
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   hw/pci/pcie_sriov.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> index 5ef8950940..a1fe65f5d8 100644
> --- a/hw/pci/pcie_sriov.c
> +++ b/hw/pci/pcie_sriov.c
> @@ -178,7 +178,6 @@ static void register_vfs(PCIDevice *dev)
>       num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
>   
>       dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
> -    assert(dev->exp.sriov_pf.vf);
>   
>       trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
>                                PCI_FUNC(dev->devfn), num_vfs);


Re: [PATCH] pcie_sriov: Remove g_new assertion
Posted by YangHang Liu 12 months ago
After applying this patch, the VM with a igbvf will not crash during reboot.

Tested-by: Yanghang Liu<yanghliu@redhat.com>



On Mon, Nov 27, 2023 at 5:12 PM Cédric Le Goater <clg@redhat.com> wrote:

> On 11/23/23 08:56, Akihiko Odaki wrote:
> > g_new() aborts if the allocation fails so it returns NULL only if the
> > requested allocation size is zero. register_vfs() makes such an
> > allocation if NumVFs is zero so it should not assert that g_new()
> > returns a non-NULL value.
> >
> > Fixes: 7c0fa8dff8 ("pcie: Add support for Single Root I/O Virtualization
> (SR/IOV)")
> > Buglink: https://issues.redhat.com/browse/RHEL-17209
> > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>
>
> Reviewed-by: Cédric Le Goater <clg@redhat.com>
>
> Thanks,
>
> C.
>
>
> > ---
> >   hw/pci/pcie_sriov.c | 1 -
> >   1 file changed, 1 deletion(-)
> >
> > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> > index 5ef8950940..a1fe65f5d8 100644
> > --- a/hw/pci/pcie_sriov.c
> > +++ b/hw/pci/pcie_sriov.c
> > @@ -178,7 +178,6 @@ static void register_vfs(PCIDevice *dev)
> >       num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
> >
> >       dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
> > -    assert(dev->exp.sriov_pf.vf);
> >
> >       trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
> >                                PCI_FUNC(dev->devfn), num_vfs);
>
>
>
Re: [PATCH] pcie_sriov: Remove g_new assertion
Posted by Cédric Le Goater 12 months ago
On 11/30/23 10:52, YangHang Liu wrote:
> After applying this patch, the VM with a igbvf will not crash during reboot.
> 
> Tested-by: Yanghang Liu<yanghliu@redhat.com <mailto:yanghliu@redhat.com>>

Michael, do you have plans to send a PR for -rc3 ?

Thanks,

C.



> 
> On Mon, Nov 27, 2023 at 5:12 PM Cédric Le Goater <clg@redhat.com <mailto:clg@redhat.com>> wrote:
> 
>     On 11/23/23 08:56, Akihiko Odaki wrote:
>      > g_new() aborts if the allocation fails so it returns NULL only if the
>      > requested allocation size is zero. register_vfs() makes such an
>      > allocation if NumVFs is zero so it should not assert that g_new()
>      > returns a non-NULL value.
>      >
>      > Fixes: 7c0fa8dff8 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)")
>      > Buglink: https://issues.redhat.com/browse/RHEL-17209 <https://issues.redhat.com/browse/RHEL-17209>
>      > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com <mailto:akihiko.odaki@daynix.com>>
> 
> 
>     Reviewed-by: Cédric Le Goater <clg@redhat.com <mailto:clg@redhat.com>>
> 
>     Thanks,
> 
>     C.
> 
> 
>      > ---
>      >   hw/pci/pcie_sriov.c | 1 -
>      >   1 file changed, 1 deletion(-)
>      >
>      > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
>      > index 5ef8950940..a1fe65f5d8 100644
>      > --- a/hw/pci/pcie_sriov.c
>      > +++ b/hw/pci/pcie_sriov.c
>      > @@ -178,7 +178,6 @@ static void register_vfs(PCIDevice *dev)
>      >       num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
>      >
>      >       dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
>      > -    assert(dev->exp.sriov_pf.vf);
>      >
>      >       trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
>      >                                PCI_FUNC(dev->devfn), num_vfs);
> 
> 


Re: [PATCH] pcie_sriov: Remove g_new assertion
Posted by Michael S. Tsirkin 12 months ago
On Thu, Nov 30, 2023 at 04:37:43PM +0100, Cédric Le Goater wrote:
> On 11/30/23 10:52, YangHang Liu wrote:
> > After applying this patch, the VM with a igbvf will not crash during reboot.
> > 
> > Tested-by: Yanghang Liu<yanghliu@redhat.com <mailto:yanghliu@redhat.com>>
> 
> Michael, do you have plans to send a PR for -rc3 ?
> 
> Thanks,
> 
> C.


Yes.

> 
> 
> > 
> > On Mon, Nov 27, 2023 at 5:12 PM Cédric Le Goater <clg@redhat.com <mailto:clg@redhat.com>> wrote:
> > 
> >     On 11/23/23 08:56, Akihiko Odaki wrote:
> >      > g_new() aborts if the allocation fails so it returns NULL only if the
> >      > requested allocation size is zero. register_vfs() makes such an
> >      > allocation if NumVFs is zero so it should not assert that g_new()
> >      > returns a non-NULL value.
> >      >
> >      > Fixes: 7c0fa8dff8 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)")
> >      > Buglink: https://issues.redhat.com/browse/RHEL-17209 <https://issues.redhat.com/browse/RHEL-17209>
> >      > Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com <mailto:akihiko.odaki@daynix.com>>
> > 
> > 
> >     Reviewed-by: Cédric Le Goater <clg@redhat.com <mailto:clg@redhat.com>>
> > 
> >     Thanks,
> > 
> >     C.
> > 
> > 
> >      > ---
> >      >   hw/pci/pcie_sriov.c | 1 -
> >      >   1 file changed, 1 deletion(-)
> >      >
> >      > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> >      > index 5ef8950940..a1fe65f5d8 100644
> >      > --- a/hw/pci/pcie_sriov.c
> >      > +++ b/hw/pci/pcie_sriov.c
> >      > @@ -178,7 +178,6 @@ static void register_vfs(PCIDevice *dev)
> >      >       num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
> >      >
> >      >       dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
> >      > -    assert(dev->exp.sriov_pf.vf);
> >      >
> >      >       trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
> >      >                                PCI_FUNC(dev->devfn), num_vfs);
> > 
> > 


Re: [PATCH-for-8.2] pcie_sriov: Remove g_new assertion
Posted by Philippe Mathieu-Daudé 1 year ago
On 23/11/23 08:56, Akihiko Odaki wrote:
> g_new() aborts if the allocation fails so it returns NULL only if the
> requested allocation size is zero. register_vfs() makes such an
> allocation if NumVFs is zero so it should not assert that g_new()
> returns a non-NULL value.
> 
> Fixes: 7c0fa8dff8 ("pcie: Add support for Single Root I/O Virtualization (SR/IOV)")
> Buglink: https://issues.redhat.com/browse/RHEL-17209
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>   hw/pci/pcie_sriov.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> index 5ef8950940..a1fe65f5d8 100644
> --- a/hw/pci/pcie_sriov.c
> +++ b/hw/pci/pcie_sriov.c
> @@ -178,7 +178,6 @@ static void register_vfs(PCIDevice *dev)
>       num_vfs = pci_get_word(dev->config + sriov_cap + PCI_SRIOV_NUM_VF);
>   
>       dev->exp.sriov_pf.vf = g_new(PCIDevice *, num_vfs);
> -    assert(dev->exp.sriov_pf.vf);
>   
>       trace_sriov_register_vfs(dev->name, PCI_SLOT(dev->devfn),
>                                PCI_FUNC(dev->devfn), num_vfs);

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>