[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/4] qdev: support to get a device firmware path
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH 1/4] qdev: support to get a device firmware path directly |
Date: |
Wed, 21 Jan 2015 11:52:32 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 |
On 21/01/2015 03:14, Gonglei wrote:
> On 2015/1/21 0:10, Paolo Bonzini wrote:
>
>>
>>
>> On 19/01/2015 14:23, address@hidden wrote:
>>> @@ -780,6 +788,12 @@ static int qdev_get_fw_dev_path_helper(DeviceState
>>> *dev, char *p, int size)
>>> d = bus_get_fw_dev_path(dev->parent_bus, dev);
>>> }
>>> if (d) {
>>> + l += snprintf(p + l, size - l, "%s/", d);
>>> + g_free(d);
>>> + }
>>> +
>>> + d = qdev_get_own_fw_dev_path_from_handler(dev->parent_bus, dev);
>>
>> This changes preexisting behavior. If d was true, you wouldn't go down
>> the following else. Now it does.
>>
>
> On the face of things, it is. But actually they are equal. Please notice I
> added a "/" at the
> end p, and then the function can return if d was NULL.
> l += snprintf(p + l, size - l, "%s/", d);
But in this case I think the "return l" should become unconditional.
It should move out of the "else".
>> I was thinking it should be handled though the "suffix" argument to
>> add_boot_device_path, but that's harder now that the suffix has to be
>> passed to device_add_bootindex_property.
>>
>
> Yes.
>
>> Perhaps you could call qdev_get_own_fw_dev_path_from_handler in
>> get_boot_devices_list, and convert non-NULL suffixes to implementations
>> of FWPathProvider?
>
> Maybe your meaning is "convert NULL suffixes to implementations
> of FWPathProvider"? Something like below:
No, I meant non-NULL.
In the beginning it can be something like this:
diff --git a/bootdevice.c b/bootdevice.c
index 5914417..916bfb7 100644
--- a/bootdevice.c
+++ b/bootdevice.c
@@ -210,7 +210,8 @@ char *get_boot_devices_list(size_t *size, bool
ignore_suffixes)
char *list = NULL;
QTAILQ_FOREACH(i, &fw_boot_order, link) {
- char *devpath = NULL, *bootpath;
+ char *devpath = NULL, *suffix = NULL;
+ char *bootpath;
size_t len;
if (i->dev) {
@@ -218,21 +219,22 @@ char *get_boot_devices_list(size_t *size, bool
ignore_suffixes)
assert(devpath);
}
- if (i->suffix && !ignore_suffixes && devpath) {
- size_t bootpathlen = strlen(devpath) + strlen(i->suffix) + 1;
-
- bootpath = g_malloc(bootpathlen);
- snprintf(bootpath, bootpathlen, "%s%s", devpath, i->suffix);
- g_free(devpath);
- } else if (devpath) {
- bootpath = devpath;
- } else if (!ignore_suffixes) {
- assert(i->suffix);
- bootpath = g_strdup(i->suffix);
- } else {
- bootpath = g_strdup("");
+ if (!ignore_suffixes) {
+ d = qdev_get_own_fw_dev_path_from_handler(i->dev->parent_bus,
i->dev);
+ if (d) {
+ assert(!i->suffix);
+ suffix = d;
+ } else {
+ suffix = g_strdup(i->suffix);
+ }
}
+ bootpath = g_strdup_printf("%s%s",
+ devpath ? devpath : "",
+ suffix ? suffix : "");
+ g_free(devpath);
+ g_free(suffix);
+
if (total) {
list[total-1] = '\n';
}
Then as time permits the suffix can be phased out and replaced by
FWPathProvider on the device.
Paolo
> But I feel this more complicated. Isn't ?
>
> Regards,
> -Gonglei
>
>> Paolo
>>
>>> + if (d) {
>>> l += snprintf(p + l, size - l, "%s", d);
>>> g_free(d);
>>> } else {
>
>
>
>
>
[Qemu-devel] [PATCH 2/4] vhost-scsi: add bootindex property, arei.gonglei, 2015/01/19
[Qemu-devel] [PATCH 3/4] vhost-scsi: realize the TYPE_FW_PATH_PROVIDER interface, arei.gonglei, 2015/01/19
[Qemu-devel] [PATCH 4/4] vhost-scsi: add an ioctl interface to get target id, arei.gonglei, 2015/01/19
- Re: [Qemu-devel] [PATCH 4/4] vhost-scsi: add an ioctl interface to get target id, Michael S. Tsirkin, 2015/01/19
- Re: [Qemu-devel] [PATCH 4/4] vhost-scsi: add an ioctl interface to get target id, Gonglei, 2015/01/26
- Re: [Qemu-devel] [PATCH 4/4] vhost-scsi: add an ioctl interface to get target id, Paolo Bonzini, 2015/01/26
- Re: [Qemu-devel] [PATCH 4/4] vhost-scsi: add an ioctl interface to get target id, Gonglei, 2015/01/26
- Re: [Qemu-devel] [PATCH 4/4] vhost-scsi: add an ioctl interface to get target id, Paolo Bonzini, 2015/01/26
- Re: [Qemu-devel] [PATCH 4/4] vhost-scsi: add an ioctl interface to get target id, Gonglei, 2015/01/26