[libvirt] [PATCH v2 2/2] qemu_process: detect updated video ram size values from QEMU

[Pavel Hrdina]

On 01/09/2015 02:15 PM, Daniel P. Berrange wrote:
> On Fri, Jan 09, 2015 at 02:08:06PM +0100, Pavel Hrdina wrote:
>> On 01/09/2015 10:51 AM, Daniel P. Berrange wrote:
>>> On Fri, Jan 09, 2015 at 10:02:07AM +0100, Pavel Hrdina wrote:
>>>> QEMU internally updates the size of video memory if the domain XML had
>>>> provided too low memory size or there are some dependencies for a QXL
>>>> devices 'vgamem' and 'ram' size. We need to know about the changes and
>>>> store them into the status XML to not break migration or managedsave
>>>> through different libvirt versions.
>>>>
>>>> Signed-off-by: Pavel Hrdina <phrdina at redhat.com>
>>>> ---
>>>>   src/qemu/qemu_monitor.c      | 38 ++++++++++++++++++++++++
>>>>   src/qemu/qemu_monitor.h      |  4 +++
>>>>   src/qemu/qemu_monitor_json.c | 69 ++++++++++++++++++++++++++++++++++++++++++
>>>>   src/qemu/qemu_monitor_json.h |  3 ++
>>>>   src/qemu/qemu_process.c      | 71 ++++++++++++++++++++++++++++++++++++++++++++
>>>>   5 files changed, 185 insertions(+)
>>>>
>>>> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
>>>> index 55f07f3..45bb62f 100644
>>>> --- a/src/qemu/qemu_monitor.c
>>>> +++ b/src/qemu/qemu_monitor.c
>>>> @@ -1154,6 +1154,44 @@ qemuMonitorFindBalloonObjectPath(qemuMonitorPtr mon,
>>>>       return ret;
>>>>   }
>>>>
>>>> +
>>>> +/**
>>>> + * To update video memory size in status XML we need to load correct values from
>>>> + * QEMU.  This is supported only with JSON monitor.
>>>> + *
>>>> + * Returns 0 on success, -1 on failure and sets proper error message.
>>>> + */
>>>> +int
>>>> +qemuMonitorUpdateVideoMemorySize(qemuMonitorPtr mon,
>>>> +                                 virDomainVideoDefPtr video,
>>>> +                                 const char *videoName)
>>>> +{
>>>> +    int ret = -1;
>>>> +    char *path = NULL;
>>>> +
>>>> +    if (mon->json) {
>>>> +        if (qemuMonitorFindObjectPath(mon, "/", videoName, &path) < 0) {
>>>> +            virReportError(VIR_ERR_INTERNAL_ERROR,
>>>> +                           _("Failed to find QOM Object path for device '%s'"),
>>>> +                           videoName);
>>>> +            return -1;
>>>> +        }
>>>> +        if (qemuMonitorJSONUpdateVideoMemorySize(mon, video, path) < 0)
>>>> +            goto cleanup;
>>>> +    } else {
>>>> +        virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
>>>> +                       _("JSON monitor is required"));
>>>> +        return -1;
>>>> +    }
>>>
>>> This fatal error is going to break all guest startup with non-json
>>> monitor IIUC.
>>>
>>> Regards,
>>> Daniel
>>>
>>
>> Yes I know that. Should we return 0 and still reporting that error or should
>> we just skip it at all if we cannot use mon-json?
>
> We should silently skip this when json isn't available. That code path
> will only happen on very old QEMU, which presumably won't suffer from
> the bug you're trying to fix here.
>
> Regards,
> Daniel
>

OK, thanks for review. I'll send v3.

Pavel