qemu-devel
[Top][All Lists]
Advanced

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

Re: [PATCH qemu v10] spapr: Implement Open Firmware client interface


From: BALATON Zoltan
Subject: Re: [PATCH qemu v10] spapr: Implement Open Firmware client interface
Date: Mon, 7 Dec 2020 11:33:02 +0100 (CET)

On Mon, 7 Dec 2020, Greg Kurz wrote:
On Mon, 7 Dec 2020 18:33:34 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

[...]

+    }
+
+    return offset;
+}
+
+static uint32_t of_client_finddevice(const void *fdt, uint32_t nodeaddr)
+{
+    char *node, *unit, *part;

If you do this:

     g_autofree *node = NULL, *unit = NULL, *part = NULL;


Did you mean
        g_autofree char *node = NULL, *unit = NULL, *part = NULL;
?


Yes. Sorry for the missing char :)

[...]

+static uint32_t spapr_of_client_open(SpaprMachineState *spapr, const char 
*path)
+{
+    int offset;
+    uint32_t ret = 0;
+    SpaprOfInstance *inst = NULL;
+    char *node, *unit, *part;
+
+    if (spapr->of_instance_last == 0xFFFFFFFF) {
+        /* We do not recycle ihandles yet */
+        goto trace_exit;

And g_free() is passed uninitialized pointers.

A typical use case for the g_auto magic.

g_autofree, you mean?


If you switch to g_strsplit_set(), you'd need a g_auto(GStrv). But since you
explained that you'd rather keep split_path() as is, then you should use
g_autofree indeed.

[...]


Thanks! I'll repost in a sec. But I still wonder on what terms this is
going to be allowed in the QEMU tree at all.


Pros:

This notably improves the boot time of the -kernel/-initrd experience.
Even if the feature isn't used in production, it can be quite useful for
developers.

Cons:

It doesn't provide a full alternative to SLOF with respect to the boot
loader.

This is still a lot of code that we'll need to support, especially since
the changes affect paths used in production.


IMHO, unless this gets momentum in the community, and eventually
allows to get rid of SLOF once and for all, it seems premature to
merge this.

I think it's hard to gain momentum if it's kept outside of the tree because few people will take the hassle to apply patch and compile it to try it, compared to testing it if it's already available in master. So unless it breaks something I think it would be better to merge it as an experimental feature than wait for it to mature without anyone else trying it apart from a few people. If it's in master more people could test it and maybe even sent patches or at least reports on what's more needed that does not seem to happen with patch only on the mailing list.

Regards,
BALATON Zoltan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]