Re: [Qemu-devel] [RFC 00/14] More patches to disable stuff

[Juan Quintela]

Thomas Huth <address@hidden> wrote:
> On 17.07.2018 14:04, Juan Quintela wrote:
>> Hi
>> 
>> Notice that this is an RFC because they don't work.  As said on my
>> previous submmision, we need <foo>-softmmu/config-devices.h to make
>> this work.  This series just allow us to disable the devices, but not
>> to enable it back O:-)
>> 
>> Notice:
>> 
>> - scsi stuff: we are testing they in cdrom-test.c, so we need to be
>>   able to config them out.  Notice also that #ifdefs only go in tests/<...>
>> 
>> - virtio stuff: see how we need to also change hw/virtio/virtio-pci.c
>>   to disable it.  The problem appears in the device-instropect-test.c.
>>   As they are defined in the binary, but not complied in.  We can
>>   change for a registration appreach, but that is more work that what
>>   I intended for this series.
>> 
>> What do you think?
>
> I think this is the wrong way to go. If you add #ifdefs to the sources,
> you have to make the binaries target-specific. Currently each test
> binary can work for each target architecture. With #ifdefs, that's not
> possible anymore. So please don't do that.

As the system goes now, you have something enabled if it is enabled for
any _configuration_, that is what config-all-devices.mak is supposed to
do.

> If you want to make the tests more flexible for configuration, please
> use QOM instead to check whether the devices are available or not.

QOM is the problem, not the solution (TM).  Uninteresting bits deleted.

tests/device-instrospect-test.c

static void test_device_intro_concrete(void)
{
    ...
    types = device_type_list(false);
    ...
}

static QList *device_type_list(bool abstract)
{
    return qom_list_types("device", abstract);
}

static QList *qom_list_types(const char *implements, bool abstract)
{
    QDict *resp;
    QList *ret;
    QDict *args = qdict_new();

    qdict_put_bool(args, "abstract", abstract);
    if (implements) {
        qdict_put_str(args, "implements", implements);
    }
    resp = qmp("{'execute': 'qom-list-types',"
               " 'arguments': %p }", args);
    g_assert(qdict_haskey(resp, "return"));
    ret = qdict_get_qlist(resp, "return");
    qobject_ref(ret);
    qobject_unref(resp);
    return ret;
}

If I disable CONFIG_VIRTIO_RNG, then I don't compile
common-obj-$(CONFIG_VIRTIO_RNG) += virtio-rng.o

So far so good, but look at virtio-pci.c:

static void virtio_rng_pci_realize(VirtIOPCIProxy *vpci_dev, Error **errp)
{
   ...
}

static void virtio_rng_pci_class_init(ObjectClass *klass, void *data)
{
   ....
}

static void virtio_rng_initfn(Object *obj)
{
   ...
}

static const TypeInfo virtio_rng_pci_info = {
    .name          = TYPE_VIRTIO_RNG_PCI,
    .parent        = TYPE_VIRTIO_PCI,
    .instance_size = sizeof(VirtIORngPCI),
    .instance_init = virtio_rng_initfn,
    .class_init    = virtio_rng_pci_class_init,
};

static void virtio_pci_register_types(void)
{
    type_register_static(&virtio_rng_pci_info);
...
}

See, we have defined the device "virtio-rng-pci", but there is no
implementation.  WHen I run device-intronspection-test on that qemu with
CONFIG_VIRTIO_RNG, it fails to run.  If we can agree that something is
wrong, then we can search for a solution.  I send this patches as an RFC
to ask for what people think is the best solution, or if we should even
bother in fix that.

Later, JUan.