Intended purpose of virtio-rng's QOM link "rng"?

Intended purpose of virtio-rng's QOM link "rng"?

[Markus Armbruster]

What is the intended purpose of object_property_set_link() in

    static void virtio_rng_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
    {
        VirtIORngPCI *vrng = VIRTIO_RNG_PCI(vpci_dev);
        DeviceState *vdev = DEVICE(&vrng->vdev);

        if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) {
            return;
        }

        object_property_set_link(OBJECT(vrng), "rng", OBJECT(vrng->vdev.conf.rng),
                                 NULL);
    }

?

I'm asking because the function *always* fails.  I believe it's been
failing for years.

Similar code in the CCW buddy.  Also virtio-crypto-pci and
virtio-crypto-ccw link "cryptodev".

I tried moving it before qdev_realize(), where it doesn't fail.  But
then *I* fail, namely at finding any effect in QOM.

If it's really useless, I'll send a patch to drop it.

If it has a use, tell me more, so I can test my patch to move it.

Re: Intended purpose of virtio-rng's QOM link "rng"?

[Michael S. Tsirkin]

On Mon, Jul 20, 2020 at 05:07:08PM +0200, Markus Armbruster wrote:
> What is the intended purpose of object_property_set_link() in
> 
>     static void virtio_rng_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>     {
>         VirtIORngPCI *vrng = VIRTIO_RNG_PCI(vpci_dev);
>         DeviceState *vdev = DEVICE(&vrng->vdev);
> 
>         if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) {
>             return;
>         }
> 
>         object_property_set_link(OBJECT(vrng), "rng", OBJECT(vrng->vdev.conf.rng),
>                                  NULL);
>     }
> 
> ?
> 
> I'm asking because the function *always* fails.  I believe it's been
> failing for years.
> 
> Similar code in the CCW buddy.  Also virtio-crypto-pci and
> virtio-crypto-ccw link "cryptodev".
> 
> I tried moving it before qdev_realize(), where it doesn't fail.  But
> then *I* fail, namely at finding any effect in QOM.

I suspect that's because there's already a link with that name
created by virtio core. Is that right?

> If it's really useless, I'll send a patch to drop it.
> 
> If it has a use, tell me more, so I can test my patch to move it.

Re: Intended purpose of virtio-rng's QOM link "rng"?

[Markus Armbruster]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Mon, Jul 20, 2020 at 05:07:08PM +0200, Markus Armbruster wrote:
>> What is the intended purpose of object_property_set_link() in
>> 
>>     static void virtio_rng_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
>>     {
>>         VirtIORngPCI *vrng = VIRTIO_RNG_PCI(vpci_dev);
>>         DeviceState *vdev = DEVICE(&vrng->vdev);
>> 
>>         if (!qdev_realize(vdev, BUS(&vpci_dev->bus), errp)) {
>>             return;
>>         }
>> 
>>         object_property_set_link(OBJECT(vrng), "rng", OBJECT(vrng->vdev.conf.rng),
>>                                  NULL);
>>     }
>> 
>> ?
>> 
>> I'm asking because the function *always* fails.  I believe it's been
>> failing for years.
>> 
>> Similar code in the CCW buddy.  Also virtio-crypto-pci and
>> virtio-crypto-ccw link "cryptodev".
>> 
>> I tried moving it before qdev_realize(), where it doesn't fail.  But
>> then *I* fail, namely at finding any effect in QOM.
>
> I suspect that's because there's already a link with that name
> created by virtio core. Is that right?

Looks like it, in virtio_rng_device_realize():

    if (vrng->conf.rng == NULL) {
        ... make up @default_backend ...
        object_property_set_link(OBJECT(dev), "rng", default_backend,
                                 &error_abort);
    }

The object_property_set_link actually sets vrng->vdev.conf.rng (but it
takes quite some digging to see that; MICAHI[*] at work).

For virtio-crypto, the link is vcrypto->vdev.conf.cryptodev.
virtio_crypto_pci_realize() first checks its already set:

    if (vcrypto->vdev.conf.cryptodev == NULL) {
        error_setg(errp, "'cryptodev' parameter expects a valid object");
        return;
    }

>> If it's really useless, I'll send a patch to drop it.

All four are useless; will send patches.

>> If it has a use, tell me more, so I can test my patch to move it.

Thanks!


[*] Make It Complicated And Hide It.