Re: [Qemu-devel] [PATCH v4 08/10] qemu-ga: call Windows VSS requester in fsfreeze command handler

[Tomoki Sekiyama]

On 7/1/13 9:29 , "Laszlo Ersek" <address@hidden> wrote:

>>+error:
>> +    qmp_guest_fsfreeze_thaw(NULL);
>
>Passing NULL here as "errp" concerns me slightly. I've been assuming
>that "errp" is never NULL due to the outermost QMP layer always wanting
>to receive errors and to serialize them.
>
>Specifically, a NULL "errp" would turn all error_set*(errp, ...) calls
>into no-ops under qmp_guest_fsfreeze_thaw(), and all error_is_set(errp)
>questions would answer with false. That said, nothing seems to be
>affected right now.
>
>Maybe a dummy local variable would be more future-proof... OTOH it would
>be stylistically questionable :)

OK, then it should be:
    Error *local_err = NULL;
...
error:
    qmp_guest_fsfreeze_thaw(&local_err);
    if (error_is_set(&local_err)) {
        error_free(local_err);
    }

>>+#ifdef HAS_VSS_SDK
>> +#include "qga/vss-win32-provider.h"
>> +#include "qga/vss-win32-requester.h"
>> +#endif
>
>The protection of #include "qga/vss-win32-requester.h" is inconsistent
>between "commands-win32.c" and "qga/main.c". For now the header only
>brings in a few prototypes, which shouldn't cause trouble in
>"commands-win32.c" even without VSS. Still consistent guarding would be
>nice.

I will remove #ifdef's from qga/main.c. Instead, I will add static inline
functions that does nothing when CONFIG_VSS_SDK isn't defined.
(And vss-win32-provider.h can also be removed by stop linking the DLL.)


>... This patch made me look at ga_command_state_cleanup_all().
>Apparently the POSIX flavor thaws filesystems if qemu-ga exits before a
>thaw command over QMP; see guest_fsfreeze_cleanup() and
>ga_command_state_init(). Do you think something similar would be useful
>for the Windows flavor as well?

Hm, however the backup is automatically terminated if IVssBackupComponents
is released, it would be nice to have cleanup. I will add them.

Thanks,
Tomoki Sekiyama