[Qemu-devel] [resend PATCH v2] qga-win: add support for qmp_guest_fsfreeze_freeze_list

2018-09-18 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch add support for freeze specified fs.

The valid mountpoints list member are [1]:

  The path of a mounted folder, for example, Y:\MountX\
  A drive letter, for example, D:\
  A volume GUID path of the form \\?\Volume{GUID}\,
  where GUID identifies the volume
  A UNC path that specifies a remote file share,
  for example, \\Clusterx\Share1\

[1] 
https://docs.microsoft.com/en-us/windows/desktop/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
v2:
  optimize internal logic blocks

 qga/commands-win32.c| 21 -
 qga/main.c  |  2 +-
 qga/vss-win32.c |  5 +-
 qga/vss-win32.h |  3 +-
 qga/vss-win32/requester.cpp | 92 ++---
 qga/vss-win32/requester.h   | 13 --
 6 files changed, 91 insertions(+), 45 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 98d9735389..1d627f73c1 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -776,6 +776,13 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
  * The frozen state is limited for up to 10 seconds by VSS.
  */
 int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+{
+return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
+}
+
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+   strList *mountpoints,
+   Error **errp)
 {
 int i;
 Error *local_err = NULL;
@@ -790,7 +797,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
 /* cannot risk guest agent blocking itself on a write in this state */
 ga_set_frozen(ga_state);
 
-qga_vss_fsfreeze(, true, _err);
+qga_vss_fsfreeze(, true, mountpoints, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 goto error;
@@ -808,15 +815,6 @@ error:
 return 0;
 }
 
-int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
-   strList *mountpoints,
-   Error **errp)
-{
-error_setg(errp, QERR_UNSUPPORTED);
-
-return 0;
-}
-
 /*
  * Thaw local file systems using Volume Shadow-copy Service.
  */
@@ -829,7 +827,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
 return 0;
 }
 
-qga_vss_fsfreeze(, false, errp);
+qga_vss_fsfreeze(, false, NULL, errp);
 
 ga_unset_frozen(ga_state);
 return i;
@@ -1646,7 +1644,6 @@ GList *ga_command_blacklist_init(GList *blacklist)
 "guest-set-vcpus",
 "guest-get-memory-blocks", "guest-set-memory-blocks",
 "guest-get-memory-block-size",
-"guest-fsfreeze-freeze-list",
 NULL};
 char **p = (char **)list_unsupported;
 
diff --git a/qga/main.c b/qga/main.c
index 6d70242d05..13bfff5f0a 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -151,7 +151,7 @@ static void quit_handler(int sig)
 WaitForSingleObject(hEventTimeout, 0);
 CloseHandle(hEventTimeout);
 }
-qga_vss_fsfreeze(, false, );
+qga_vss_fsfreeze(, false, NULL, );
 if (err) {
 g_debug("Error unfreezing filesystems prior to exiting: %s",
 error_get_pretty(err));
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index a541f3ae01..f444a25a70 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -147,7 +147,8 @@ void ga_uninstall_vss_provider(void)
 }
 
 /* Call VSS requester and freeze/thaw filesystems and applications */
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp)
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+  strList *mountpoints, Error **errp)
 {
 const char *func_name = freeze ? "requester_freeze" : "requester_thaw";
 QGAVSSRequesterFunc func;
@@ -164,5 +165,5 @@ void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error 
**errp)
 return;
 }
 
-func(nr_volume, );
+func(nr_volume, mountpoints, );
 }
diff --git a/qga/vss-win32.h b/qga/vss-win32.h
index 4f8e39aa5c..ce2abe5a72 100644
--- a/qga/vss-win32.h
+++ b/qga/vss-win32.h
@@ -22,6 +22,7 @@ bool vss_initialized(void);
 int ga_install_vss_provider(void);
 void ga_uninstall_vss_provider(void);
 
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp);
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+  strList *mountpints, Error **errp);
 
 #endif
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 3d9c9716c0..5378c55d23 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -234,7 +234,7 @@ out:
 }
 }
 
-void requester_freeze(int *num_vols, ErrorSet *errset)
+void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
 {
 COMPointer pAsync;
 HANDLE volume;
@@ -246,6 +246,7 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
 WCHAR 

[Qemu-devel] [PATCH v2] qga-win: add support for qmp_guest_fsfreeze_freeze_list

2018-08-31 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch add support for freeze specified fs.

The valid mountpoints list member are [1]:

  The path of a mounted folder, for example, Y:\MountX\
  A drive letter, for example, D:\
  A volume GUID path of the form \\?\Volume{GUID}\,
  where GUID identifies the volume
  A UNC path that specifies a remote file share,
  for example, \\Clusterx\Share1\

[1] 
https://docs.microsoft.com/en-us/windows/desktop/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
v2:
  optimize internal logic blocks

 qga/commands-win32.c| 21 -
 qga/main.c  |  2 +-
 qga/vss-win32.c |  5 +-
 qga/vss-win32.h |  3 +-
 qga/vss-win32/requester.cpp | 92 ++---
 qga/vss-win32/requester.h   | 13 --
 6 files changed, 91 insertions(+), 45 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 98d9735389..1d627f73c1 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -776,6 +776,13 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
  * The frozen state is limited for up to 10 seconds by VSS.
  */
 int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+{
+return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
+}
+
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+   strList *mountpoints,
+   Error **errp)
 {
 int i;
 Error *local_err = NULL;
@@ -790,7 +797,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
 /* cannot risk guest agent blocking itself on a write in this state */
 ga_set_frozen(ga_state);
 
-qga_vss_fsfreeze(, true, _err);
+qga_vss_fsfreeze(, true, mountpoints, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 goto error;
@@ -808,15 +815,6 @@ error:
 return 0;
 }
 
-int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
-   strList *mountpoints,
-   Error **errp)
-{
-error_setg(errp, QERR_UNSUPPORTED);
-
-return 0;
-}
-
 /*
  * Thaw local file systems using Volume Shadow-copy Service.
  */
@@ -829,7 +827,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
 return 0;
 }
 
-qga_vss_fsfreeze(, false, errp);
+qga_vss_fsfreeze(, false, NULL, errp);
 
 ga_unset_frozen(ga_state);
 return i;
@@ -1646,7 +1644,6 @@ GList *ga_command_blacklist_init(GList *blacklist)
 "guest-set-vcpus",
 "guest-get-memory-blocks", "guest-set-memory-blocks",
 "guest-get-memory-block-size",
-"guest-fsfreeze-freeze-list",
 NULL};
 char **p = (char **)list_unsupported;
 
diff --git a/qga/main.c b/qga/main.c
index 6d70242d05..13bfff5f0a 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -151,7 +151,7 @@ static void quit_handler(int sig)
 WaitForSingleObject(hEventTimeout, 0);
 CloseHandle(hEventTimeout);
 }
-qga_vss_fsfreeze(, false, );
+qga_vss_fsfreeze(, false, NULL, );
 if (err) {
 g_debug("Error unfreezing filesystems prior to exiting: %s",
 error_get_pretty(err));
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index a541f3ae01..f444a25a70 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -147,7 +147,8 @@ void ga_uninstall_vss_provider(void)
 }
 
 /* Call VSS requester and freeze/thaw filesystems and applications */
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp)
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+  strList *mountpoints, Error **errp)
 {
 const char *func_name = freeze ? "requester_freeze" : "requester_thaw";
 QGAVSSRequesterFunc func;
@@ -164,5 +165,5 @@ void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error 
**errp)
 return;
 }
 
-func(nr_volume, );
+func(nr_volume, mountpoints, );
 }
diff --git a/qga/vss-win32.h b/qga/vss-win32.h
index 4f8e39aa5c..ce2abe5a72 100644
--- a/qga/vss-win32.h
+++ b/qga/vss-win32.h
@@ -22,6 +22,7 @@ bool vss_initialized(void);
 int ga_install_vss_provider(void);
 void ga_uninstall_vss_provider(void);
 
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp);
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+  strList *mountpints, Error **errp);
 
 #endif
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 3d9c9716c0..5378c55d23 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -234,7 +234,7 @@ out:
 }
 }
 
-void requester_freeze(int *num_vols, ErrorSet *errset)
+void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
 {
 COMPointer pAsync;
 HANDLE volume;
@@ -246,6 +246,7 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
 WCHAR 

Re: [Qemu-devel] [resend][PATCH] qga-win: add support for qmp_guest_fsfreeze_freeze_list

2018-08-30 Thread Chen Hanxiao



At 2018-08-24 04:22:08, "Michael Roth"  wrote:
>Quoting Chen Hanxiao (2018-08-09 20:13:48)
>> From: Chen Hanxiao 
>> 
>> This patch add support for freeze specified fs.
>> 
>> The valid mountpoints list member are [1]:
>> 
>>   The path of a mounted folder, for example, Y:\MountX\
>>   A drive letter, for example, D:\
>>   A volume GUID path of the form \\?\Volume{GUID}\,
>>   where GUID identifies the volume
>>   A UNC path that specifies a remote file share,
>>   for example, \\Clusterx\Share1\
>> 
>> [1] 
>> https://docs.microsoft.com/en-us/windows/desktop/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset
>> 
>> Cc: Michael Roth 
>> Signed-off-by: Chen Hanxiao 
>
>Some small nits below, but looks good otherwise.
>
>> ---
>>  qga/commands-win32.c| 21 ++
>>  qga/main.c  |  2 +-
[...]
>> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
>> index 3d9c9716c0..0bd170eddc 100644
>> --- a/qga/vss-win32/requester.cpp
>> +++ b/qga/vss-win32/requester.cpp
>> @@ -234,7 +234,7 @@ out:
>>  }
>>  }
>> 
>> -void requester_freeze(int *num_vols, ErrorSet *errset)
>> +void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
>
>Does this need to be declared as void *, or can we use the direct VolList*
>type? Maybe even const VolList * const, since it is read-only?

That may change the current structure a lot.
The caller qga_vss_fsfreeze also serve requester_thaw.
We'd better keep its declaration as it used to be.

>
>>  {
>>  COMPointer pAsync;
>>  HANDLE volume;
>> @@ -245,7 +245,9 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
>>  SECURITY_ATTRIBUTES sa;
>>  WCHAR short_volume_name[64], *display_name = short_volume_name;
>>  DWORD wait_status;
>> +PWCHAR WStr = NULL;
>>  int num_fixed_drives = 0, i;
>> +int num_mount_points = 0;
>> 
>>  if (vss_ctx.pVssbc) { /* already frozen */
>>  *num_vols = 0;
>> @@ -337,12 +339,42 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
>>  goto out;
>>  }
>> 
>> -volume = FindFirstVolumeW(short_volume_name, sizeof(short_volume_name));
>> -if (volume == INVALID_HANDLE_VALUE) {
>> -err_set(errset, hr, "failed to find first volume");
>> +if (mountpoints) {
>> +for (volList *list = (volList *)mountpoints; list; list = 
>> list->next) {
>> +size_t len = strlen(list->value) + 1;
>> +size_t converted = 0;
>> +VSS_ID pid;
>> +
>> +WStr = new wchar_t[len];
>> +mbstowcs_s(, WStr, len, list->value, _TRUNCATE);
>> +
>> +hr = vss_ctx.pVssbc->AddToSnapshotSet(WStr,
>> +  g_gProviderId, );
>> +if (FAILED(hr)) {
>> +err_set(errset, hr, "failed to add %S to snapshot set", 
>> WStr);
>> +goto out;
>
>WStr is only needed within this block, so I'd prefer declaring it here
>and then just adding an additional "delete WStr;" before the "goto out;".
>
>Also I know the APIs force us to differ from the QEMU coding guidelines
>with regard to CamelCase and whatnot, but I'd still prefer we stick to
>it when possible. WStr also isn't very descriptive, maybe
>volume_name_wchar?

Sure, will be fixed in v2.

>
>> +}
>> +num_mount_points++;
>> +
>> +delete WStr;
>> +}
>> +}
>> +
>> +if (mountpoints && num_mount_points == 0) {
>> +/* If there is no valid mount points, just exit. */
>>  goto out;
>>  }
>
>Logic might be a little cleaner if you move this block after the for
>loop above.

Done.

>
>> -for (;;) {
>> +
>> +
>> +if (!mountpoints) {
>> +volume = FindFirstVolumeW(short_volume_name, 
>> sizeof(short_volume_name));
>> +if (volume == INVALID_HANDLE_VALUE) {
>> +err_set(errset, hr, "failed to find first volume");
>> +goto out;
>> +}
>> +}
>> +
>> +while (!mountpoints) {
>
>mountpoints is read-only in this function, so this is basically an
>unconditional loop. I'd rather we do for(;;) like it was previously
>and nest that in the "if (!mountpoints)" above to keep the logic
>more contained.

More clear.

>
>>   

[Qemu-devel] [resend][PATCH] qga-win: add support for qmp_guest_fsfreeze_freeze_list

2018-08-09 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch add support for freeze specified fs.

The valid mountpoints list member are [1]:

  The path of a mounted folder, for example, Y:\MountX\
  A drive letter, for example, D:\
  A volume GUID path of the form \\?\Volume{GUID}\,
  where GUID identifies the volume
  A UNC path that specifies a remote file share,
  for example, \\Clusterx\Share1\

[1] 
https://docs.microsoft.com/en-us/windows/desktop/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
 qga/commands-win32.c| 21 ++
 qga/main.c  |  2 +-
 qga/vss-win32.c |  5 ++--
 qga/vss-win32.h |  3 +-
 qga/vss-win32/requester.cpp | 56 +++--
 qga/vss-win32/requester.h   | 13 +++--
 6 files changed, 72 insertions(+), 28 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 98d9735389..1d627f73c1 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -776,6 +776,13 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
  * The frozen state is limited for up to 10 seconds by VSS.
  */
 int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+{
+return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
+}
+
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+   strList *mountpoints,
+   Error **errp)
 {
 int i;
 Error *local_err = NULL;
@@ -790,7 +797,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
 /* cannot risk guest agent blocking itself on a write in this state */
 ga_set_frozen(ga_state);
 
-qga_vss_fsfreeze(, true, _err);
+qga_vss_fsfreeze(, true, mountpoints, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 goto error;
@@ -808,15 +815,6 @@ error:
 return 0;
 }
 
-int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
-   strList *mountpoints,
-   Error **errp)
-{
-error_setg(errp, QERR_UNSUPPORTED);
-
-return 0;
-}
-
 /*
  * Thaw local file systems using Volume Shadow-copy Service.
  */
@@ -829,7 +827,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
 return 0;
 }
 
-qga_vss_fsfreeze(, false, errp);
+qga_vss_fsfreeze(, false, NULL, errp);
 
 ga_unset_frozen(ga_state);
 return i;
@@ -1646,7 +1644,6 @@ GList *ga_command_blacklist_init(GList *blacklist)
 "guest-set-vcpus",
 "guest-get-memory-blocks", "guest-set-memory-blocks",
 "guest-get-memory-block-size",
-"guest-fsfreeze-freeze-list",
 NULL};
 char **p = (char **)list_unsupported;
 
diff --git a/qga/main.c b/qga/main.c
index 87372d40ef..098c783f1f 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -152,7 +152,7 @@ static void quit_handler(int sig)
 WaitForSingleObject(hEventTimeout, 0);
 CloseHandle(hEventTimeout);
 }
-qga_vss_fsfreeze(, false, );
+qga_vss_fsfreeze(, false, NULL, );
 if (err) {
 g_debug("Error unfreezing filesystems prior to exiting: %s",
 error_get_pretty(err));
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index a541f3ae01..f444a25a70 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -147,7 +147,8 @@ void ga_uninstall_vss_provider(void)
 }
 
 /* Call VSS requester and freeze/thaw filesystems and applications */
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp)
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+  strList *mountpoints, Error **errp)
 {
 const char *func_name = freeze ? "requester_freeze" : "requester_thaw";
 QGAVSSRequesterFunc func;
@@ -164,5 +165,5 @@ void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error 
**errp)
 return;
 }
 
-func(nr_volume, );
+func(nr_volume, mountpoints, );
 }
diff --git a/qga/vss-win32.h b/qga/vss-win32.h
index 4f8e39aa5c..ce2abe5a72 100644
--- a/qga/vss-win32.h
+++ b/qga/vss-win32.h
@@ -22,6 +22,7 @@ bool vss_initialized(void);
 int ga_install_vss_provider(void);
 void ga_uninstall_vss_provider(void);
 
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp);
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+  strList *mountpints, Error **errp);
 
 #endif
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 3d9c9716c0..0bd170eddc 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -234,7 +234,7 @@ out:
 }
 }
 
-void requester_freeze(int *num_vols, ErrorSet *errset)
+void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
 {
 COMPointer pAsync;
 HANDLE volume;
@@ -245,7 +245,9 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
 SECURITY_ATTRIBUTES sa;
 WCHAR 

[Qemu-devel] [PATCH] qga-win: add support for qmp_guest_fsfreeze_freeze_list

2018-07-21 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch add support for freeze specified fs.

The valid mountpoints list member are [1]:

  The path of a mounted folder, for example, Y:\MountX\
  A drive letter, for example, D:\
  A volume GUID path of the form \\?\Volume{GUID}\,
  where GUID identifies the volume
  A UNC path that specifies a remote file share,
  for example, \\Clusterx\Share1\

[1] 
https://docs.microsoft.com/en-us/windows/desktop/api/vsbackup/nf-vsbackup-ivssbackupcomponents-addtosnapshotset

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
 qga/commands-win32.c| 21 ++
 qga/main.c  |  2 +-
 qga/vss-win32.c |  5 ++--
 qga/vss-win32.h |  3 +-
 qga/vss-win32/requester.cpp | 56 +++--
 qga/vss-win32/requester.h   | 13 +++--
 6 files changed, 72 insertions(+), 28 deletions(-)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 318d760a74..246f426999 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -776,6 +776,13 @@ GuestFsfreezeStatus qmp_guest_fsfreeze_status(Error **errp)
  * The frozen state is limited for up to 10 seconds by VSS.
  */
 int64_t qmp_guest_fsfreeze_freeze(Error **errp)
+{
+return qmp_guest_fsfreeze_freeze_list(false, NULL, errp);
+}
+
+int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
+   strList *mountpoints,
+   Error **errp)
 {
 int i;
 Error *local_err = NULL;
@@ -790,7 +797,7 @@ int64_t qmp_guest_fsfreeze_freeze(Error **errp)
 /* cannot risk guest agent blocking itself on a write in this state */
 ga_set_frozen(ga_state);
 
-qga_vss_fsfreeze(, true, _err);
+qga_vss_fsfreeze(, true, mountpoints, _err);
 if (local_err) {
 error_propagate(errp, local_err);
 goto error;
@@ -808,15 +815,6 @@ error:
 return 0;
 }
 
-int64_t qmp_guest_fsfreeze_freeze_list(bool has_mountpoints,
-   strList *mountpoints,
-   Error **errp)
-{
-error_setg(errp, QERR_UNSUPPORTED);
-
-return 0;
-}
-
 /*
  * Thaw local file systems using Volume Shadow-copy Service.
  */
@@ -829,7 +827,7 @@ int64_t qmp_guest_fsfreeze_thaw(Error **errp)
 return 0;
 }
 
-qga_vss_fsfreeze(, false, errp);
+qga_vss_fsfreeze(, false, NULL, errp);
 
 ga_unset_frozen(ga_state);
 return i;
@@ -1633,7 +1631,6 @@ GList *ga_command_blacklist_init(GList *blacklist)
 "guest-set-vcpus",
 "guest-get-memory-blocks", "guest-set-memory-blocks",
 "guest-get-memory-block-size",
-"guest-fsfreeze-freeze-list",
 NULL};
 char **p = (char **)list_unsupported;
 
diff --git a/qga/main.c b/qga/main.c
index 537cc0e162..2575ea206a 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -152,7 +152,7 @@ static void quit_handler(int sig)
 WaitForSingleObject(hEventTimeout, 0);
 CloseHandle(hEventTimeout);
 }
-qga_vss_fsfreeze(, false, );
+qga_vss_fsfreeze(, false, NULL, );
 if (err) {
 g_debug("Error unfreezing filesystems prior to exiting: %s",
 error_get_pretty(err));
diff --git a/qga/vss-win32.c b/qga/vss-win32.c
index a541f3ae01..f444a25a70 100644
--- a/qga/vss-win32.c
+++ b/qga/vss-win32.c
@@ -147,7 +147,8 @@ void ga_uninstall_vss_provider(void)
 }
 
 /* Call VSS requester and freeze/thaw filesystems and applications */
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp)
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+  strList *mountpoints, Error **errp)
 {
 const char *func_name = freeze ? "requester_freeze" : "requester_thaw";
 QGAVSSRequesterFunc func;
@@ -164,5 +165,5 @@ void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error 
**errp)
 return;
 }
 
-func(nr_volume, );
+func(nr_volume, mountpoints, );
 }
diff --git a/qga/vss-win32.h b/qga/vss-win32.h
index 4f8e39aa5c..ce2abe5a72 100644
--- a/qga/vss-win32.h
+++ b/qga/vss-win32.h
@@ -22,6 +22,7 @@ bool vss_initialized(void);
 int ga_install_vss_provider(void);
 void ga_uninstall_vss_provider(void);
 
-void qga_vss_fsfreeze(int *nr_volume, bool freeze, Error **errp);
+void qga_vss_fsfreeze(int *nr_volume, bool freeze,
+  strList *mountpints, Error **errp);
 
 #endif
diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 3d9c9716c0..0bd170eddc 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -234,7 +234,7 @@ out:
 }
 }
 
-void requester_freeze(int *num_vols, ErrorSet *errset)
+void requester_freeze(int *num_vols, void *mountpoints, ErrorSet *errset)
 {
 COMPointer pAsync;
 HANDLE volume;
@@ -245,7 +245,9 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
 SECURITY_ATTRIBUTES sa;
 WCHAR 

[Qemu-devel] [PATCH resend v2] qga: unset frozen state if no mount points are frozen

2018-06-14 Thread Chen Hanxiao
From: Chen Hanxiao 

If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
we may got nothing to freeze as all mountpoints are
not valid.
So call ga_unset_frozen in this senario.

Also, if we return 0 frozen fs, there is no need to call
guest-fsfreeze-thaw.

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
v2:
  remove has_mountpoints special case
  add qapi-schema.json section

 qga/commands-posix.c | 6 ++
 qga/qapi-schema.json | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0dc219dbcf..93bfc5b4ab 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1274,6 +1274,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
has_mountpoints,
 }
 
 free_fs_mount_list();
+/* We may not issue any FIFREEZE here.
+ * Just unset ga_state here and ready for the next call.
+ */
+if (i == 0) {
+ga_unset_frozen(ga_state);
+}
 return i;
 
 error:
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..1045cef386 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -435,7 +435,9 @@
 # for up to 10 seconds by VSS.
 #
 # Returns: Number of file systems currently frozen. On error, all filesystems
-# will be thawed.
+# will be thawed. If no filesystems are frozen as a result of this call,
+# then @guest-fsfreeze-status will remain "thawed" and calling
+# @guest-fsfreeze-thaw is not necessary.
 #
 # Since: 0.15.0
 ##
-- 
2.14.3




[Qemu-devel] [PATCH v5.2 0/2] qga: report the usage of fs in guests

2018-06-14 Thread Chen Hanxiao
This series report the usage of guest's fs
by "used_bytes" and "total_bytes" in guest-get-fsinfo.

Chen Hanxiao (2):
  qga: add mountpoint usage info to GuestFilesystemInfo
  qga-win: add driver path usage to GuestFilesystemInfo

 qga/commands-posix.c | 15 +++
 qga/commands-win32.c | 12 
 qga/qapi-schema.json |  3 +++
 3 files changed, 30 insertions(+)

-- 
2.17.1




[Qemu-devel] [PATCH v5.2 2/2] qga-win: add driver path usage to GuestFilesystemInfo

2018-06-14 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch adds support for getting the usage of
windows driver path.
The usage of fs stored as used_bytes and total_bytes.

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
v2:
  unpack usage as used-bytes and total-bytes

 qga/commands-win32.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 70ee5379f6..0103c0695c 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -670,6 +670,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, 
Error **errp)
 char fs_name[32];
 char vol_info[MAX_PATH+1];
 size_t len;
+uint64_t i64FreeBytesToCaller, i64TotalBytes, i64FreeBytes;
 GuestFilesystemInfo *fs = NULL;
 
 GetVolumePathNamesForVolumeName(guid, (LPCH), 0, _size);
@@ -699,10 +700,21 @@ static GuestFilesystemInfo *build_guest_fsinfo(char 
*guid, Error **errp)
 fs_name[sizeof(fs_name) - 1] = 0;
 fs = g_malloc(sizeof(*fs));
 fs->name = g_strdup(guid);
+fs->has_total_bytes = false;
+fs->has_used_bytes = false;
 if (len == 0) {
 fs->mountpoint = g_strdup("System Reserved");
 } else {
 fs->mountpoint = g_strndup(mnt_point, len);
+if (GetDiskFreeSpaceEx(fs->mountpoint,
+   (PULARGE_INTEGER) & i64FreeBytesToCaller,
+   (PULARGE_INTEGER) & i64TotalBytes,
+   (PULARGE_INTEGER) & i64FreeBytes)) {
+fs->used_bytes = i64TotalBytes - i64FreeBytes;
+fs->total_bytes = i64TotalBytes;
+fs->has_total_bytes = true;
+fs->has_used_bytes = true;
+}
 }
 fs->type = g_strdup(fs_name);
 fs->disk = build_guest_disk_info(guid, errp);
-- 
2.17.1




[Qemu-devel] [PATCH v5.2 1/2] qga: add mountpoint usage info to GuestFilesystemInfo

2018-06-14 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch adds support for getting the usage of mounted
filesystem.
The usage of fs stored as used_bytes and total_bytes.
It's very useful when we try to monitor guest's filesystem.

Cc: Michael Roth 
Cc: Daniel P. Berrangé 
Reviewed-by: Eric Blake 
Signed-off-by: Chen Hanxiao 
---
v2:
   add description in qapi-schema and version numbers
v3:
   use float for usage to get more precision.
v4:
   make usage a best-effort query and mark it as optional.
v5:
   report used-bytes and total-bytes in usage
v5.1:
   unpack usage as used-bytes and total-bytes
v5.2:
   remove useless assigments and fix doc

 qga/commands-posix.c | 15 +++
 qga/qapi-schema.json |  3 +++
 2 files changed, 18 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index eae817191b..e0e7993bcf 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -46,6 +46,7 @@ extern char **environ;
 #include 
 #include 
 #include 
+#include 
 
 #ifdef FIFREEZE
 #define CONFIG_FSFREEZE
@@ -1072,6 +1073,8 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
Error **errp)
 {
 GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
+struct statvfs buf;
+unsigned long used, nonroot_total, fr_size;
 char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
 mount->devmajor, mount->devminor);
 
@@ -1079,7 +1082,19 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
 fs->type = g_strdup(mount->devtype);
 build_guest_fsinfo_for_device(devpath, fs, errp);
 
+if (statvfs(fs->mountpoint, ) == 0) {
+fr_size = buf.f_frsize;
+used = buf.f_blocks - buf.f_bfree;
+nonroot_total = used + buf.f_bavail;
+fs->used_bytes = used * fr_size;
+fs->total_bytes = nonroot_total * fr_size;
+
+fs->has_total_bytes = true;
+fs->has_used_bytes = true;
+}
+
 g_free(devpath);
+
 return fs;
 }
 
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..b1bbb78c59 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -846,6 +846,8 @@
 # @name: disk name
 # @mountpoint: mount point path
 # @type: file system type string
+# @used-bytes: file system used bytes (since 3.0)
+# @total-bytes: non-root file system total bytes (since 3.0)
 # @disk: an array of disk hardware information that the volume lies on,
 #which may be empty if the disk type is not supported
 #
@@ -853,6 +855,7 @@
 ##
 { 'struct': 'GuestFilesystemInfo',
   'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
+   '*used-bytes': 'uint64', '*total-bytes': 'uint64',
'disk': ['GuestDiskAddress']} }
 
 ##
-- 
2.17.1




Re: [Qemu-devel] [PATCH v5.1 2/2] qga-win: add driver path usage to GuestFilesystemInfo

2018-06-02 Thread Chen Hanxiao

At 2018-06-02 01:15:14, "Eric Blake"  wrote:
>On 06/01/2018 11:57 AM, Chen Hanxiao wrote:
>> From: Chen Hanxiao 
...
>>   fs->mountpoint = g_strdup("System Reserved");
>>   } else {
>>   fs->mountpoint = g_strndup(mnt_point, len);
>> +if (GetDiskFreeSpaceEx(fs->mountpoint,
>> +   (PULARGE_INTEGER) & i64FreeBytesToCaller,
>> +   (PULARGE_INTEGER) & i64TotalBytes,
>> +   (PULARGE_INTEGER) & i64FreeBytes)) {
>> +fs->used_bytes = i64TotalBytes - i64FreeBytes;
>> +fs->total_bytes = i64TotalBytes;
>> +fs->has_total_bytes = true;
>> +fs->has_used_bytes = true;
>> +}
>
>I'm less familiar with Windows API, so I won't give this R-b. But now 
>that I read this patch, I do have a question: since both statvfs() and 
>GetDiskFreeSpaceEx() give you free bytes rather than used bytes, and you 
>had to perform a subtraction to report 'used-bytes' over QGA, would it 
>make any more sense to instead report 'free-bytes' over QGA for less 
>subtraction work on your end and so that the QGA interface is more 
>similar to existing interfaces?
>


All build-in tools report used-bytes for linux and windows.

For windows, GetDiskFreeSpaceEx would tell total and free bytes,
used bytes is  total - free,  which is exactly same
from the view of disk properties.

But for linux, df(1) do some calculation for showing used-bytes and free-bytes.

df(1) takes f_blocks - f_bfree for 'Used' ,
which take 'Used' + 'Available'(f_bavail)  for non-root total, 
which is different from 'Size'(f_blocks);
Then calculate  'Use%' by Used / non-root-total.

If we want get what df(3) outputs, we must report more members of struct 
statvfs  from STATVFS(3) ,
which windows does not need it.
And client had to do some calculation with if-else dealing with data from 
windows/linux.

Customs usually take 'Use%' to monitor their disks,
so I think we should report nums that can easily get the right percentage of 
usage from it,
for both linux and windows.

Regards,
- Chen


Re: [Qemu-devel] [PATCH v5 1/2] qga: add mountpoint usage to GuestFilesystemInfo

2018-06-01 Thread Chen Hanxiao


在 2018-06-01 23:47:04,"Eric Blake"  写道:
>On 06/01/2018 10:27 AM, Chen Hanxiao wrote:
>> From: Chen Hanxiao 
>> 
>> This patch adds support for getting the usage of mounted
>> filesystem.
>> The usage of fs stored as used_bytes and total_bytes.
>> It's very useful when we try to monitor guest's filesystem.
>> 
>> Cc: Michael Roth 
>> Cc: Eric Blake 
>> Cc: Daniel P. Berrangé 
>> Signed-off-by: Chen Hanxiao 
>> 
>
>> +++ b/qga/qapi-schema.json
>> @@ -840,12 +840,24 @@
>>  'bus-type': 'GuestDiskBusType',
>>  'bus': 'int', 'target': 'int', 'unit': 'int'} }
>>   
>> +##
>> +# @GuestFsUsage:
>> +#
>> +# @used-bytes:  file system used bytes
>> +# @total-bytes: file system total bytes for nonroot user
>> +#
>> +# Since: 3.0
>> +##
>> +{ 'struct': 'GuestFsUsage',
>> +  'data': {'used-bytes': 'uint64', 'total-bytes': 'uint64'} }
>
>That seems like pointless nesting on the wire, unless we have plans of 
>reusing this type in other API calls.  Is it any easier...
>
>> +
>>   ##
>>   # @GuestFilesystemInfo:
>>   #
>>   # @name: disk name
>>   # @mountpoint: mount point path
>>   # @type: file system type string
>> +# @usage: file system usage struct (since 3.0)
>>   # @disk: an array of disk hardware information that the volume lies on,
>>   #which may be empty if the disk type is not supported
>>   #
>> @@ -853,7 +865,7 @@
>>   ##
>>   { 'struct': 'GuestFilesystemInfo',
>> 'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
>> -   'disk': ['GuestDiskAddress']} }
>> +   '*usage': 'GuestFsUsage', 'disk': ['GuestDiskAddress']} }
>
>...to just inline '*used-bytes' and '*total-bytes' here?
>

I just want to group them together, and use one has_XXX.

I'll make a some fix for this.
Thanks for you advice.

Regards,
- Chen


[Qemu-devel] [PATCH v5.1 0/2] qga: report the usage of fs in guests

2018-06-01 Thread Chen Hanxiao
This series report the usage of guest's fs
by "used_bytes" and "total_bytes" in guest-get-fsinfo.

Chen Hanxiao (2):
  qga: add mountpoint usage info to GuestFilesystemInfo
  qga-win: add driver path usage to GuestFilesystemInfo

 qga/commands-posix.c | 18 ++
 qga/commands-win32.c | 12 
 qga/qapi-schema.json |  3 +++
 3 files changed, 33 insertions(+)

-- 
2.17.0




[Qemu-devel] [PATCH v5.1 2/2] qga-win: add driver path usage to GuestFilesystemInfo

2018-06-01 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch adds support for getting the usage of
windows driver path.
The usage of fs stored as used_bytes and total_bytes.

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
v2:
  unpack usage as used-bytes and total-bytes

 qga/commands-win32.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 2d48394748..eff26c177e 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -670,6 +670,7 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, 
Error **errp)
 char fs_name[32];
 char vol_info[MAX_PATH+1];
 size_t len;
+uint64_t i64FreeBytesToCaller, i64TotalBytes, i64FreeBytes;
 GuestFilesystemInfo *fs = NULL;
 
 GetVolumePathNamesForVolumeName(guid, (LPCH), 0, _size);
@@ -699,10 +700,21 @@ static GuestFilesystemInfo *build_guest_fsinfo(char 
*guid, Error **errp)
 fs_name[sizeof(fs_name) - 1] = 0;
 fs = g_malloc(sizeof(*fs));
 fs->name = g_strdup(guid);
+fs->has_total_bytes = false;
+fs->has_used_bytes = false;
 if (len == 0) {
 fs->mountpoint = g_strdup("System Reserved");
 } else {
 fs->mountpoint = g_strndup(mnt_point, len);
+if (GetDiskFreeSpaceEx(fs->mountpoint,
+   (PULARGE_INTEGER) & i64FreeBytesToCaller,
+   (PULARGE_INTEGER) & i64TotalBytes,
+   (PULARGE_INTEGER) & i64FreeBytes)) {
+fs->used_bytes = i64TotalBytes - i64FreeBytes;
+fs->total_bytes = i64TotalBytes;
+fs->has_total_bytes = true;
+fs->has_used_bytes = true;
+}
 }
 fs->type = g_strdup(fs_name);
 fs->disk = build_guest_disk_info(guid, errp);
-- 
2.17.0




[Qemu-devel] [PATCH v5.1 1/2] qga: add mountpoint usage info to GuestFilesystemInfo

2018-06-01 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch adds support for getting the usage of mounted
filesystem.
The usage of fs stored as used_bytes and total_bytes.
It's very useful when we try to monitor guest's filesystem.

Cc: Michael Roth 
Cc: Eric Blake 
Cc: Daniel P. Berrangé 
Signed-off-by: Chen Hanxiao 
---
v2:
   add description in qapi-schema and version numbers
v3:
   use float for usage to get more precision.
v4:
   make usage a best-effort query and mark it as optional.
v5:
   report used-bytes and total-bytes in usage
v5.1:
   unpack usage as used-bytes and total-bytes

 qga/commands-posix.c | 18 ++
 qga/qapi-schema.json |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0dc219dbcf..c60f10577e 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -46,6 +46,7 @@ extern char **environ;
 #include 
 #include 
 #include 
+#include 
 
 #ifdef FIFREEZE
 #define CONFIG_FSFREEZE
@@ -1072,6 +1073,8 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
Error **errp)
 {
 GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
+struct statvfs buf;
+unsigned long used, nonroot_total, fr_size;
 char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
 mount->devmajor, mount->devminor);
 
@@ -1079,7 +1082,22 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
 fs->type = g_strdup(mount->devtype);
 build_guest_fsinfo_for_device(devpath, fs, errp);
 
+if (statvfs(fs->mountpoint, )) {
+fs->has_total_bytes = false;
+fs->has_used_bytes = false;
+} else {
+fr_size = buf.f_frsize;
+used = buf.f_blocks - buf.f_bfree;
+nonroot_total = used + buf.f_bavail;
+fs->used_bytes = used * fr_size;
+fs->total_bytes = nonroot_total * fr_size;
+
+fs->has_total_bytes = true;
+fs->has_used_bytes = true;
+}
+
 g_free(devpath);
+
 return fs;
 }
 
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..56ce2d08e2 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -846,6 +846,8 @@
 # @name: disk name
 # @mountpoint: mount point path
 # @type: file system type string
+# @used-bytes: file system used bytes (since 3.0)
+# @total-bytes: nonroot file system total bytes (since 3.0)
 # @disk: an array of disk hardware information that the volume lies on,
 #which may be empty if the disk type is not supported
 #
@@ -853,6 +855,7 @@
 ##
 { 'struct': 'GuestFilesystemInfo',
   'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
+   '*used-bytes': 'uint64', '*total-bytes': 'uint64',
'disk': ['GuestDiskAddress']} }
 
 ##
-- 
2.17.0




Re: [Qemu-devel] [PATCH v4] qga: add mountpoint usage to GuestFilesystemInfo

2018-06-01 Thread Chen Hanxiao



At 2018-06-01 20:19:56, "Daniel P. Berrangé"  wrote:
>On Fri, Jun 01, 2018 at 02:11:21PM +0800, Chen Hanxiao wrote:
>> From: Chen Hanxiao 
>> 
>> This patch adds support for getting the usage of mounted
>> filesystem.
>> It's very useful when we try to monitor guest's filesystem.
>> 
>> Cc: Michael Roth 
>> Cc: Eric Blake 
>> Signed-off-by: Chen Hanxiao 
>> 
>> ---
>> v2:
>>add description in qapi-schema and version numbers
>> v3:
>>use float for usage to get more precision.
>> v4:
>>make usage as a best-effort query and mark it as optional.
>> 
>>  qga/commands-posix.c | 17 +
>>  qga/qapi-schema.json |  3 ++-
>>  2 files changed, 19 insertions(+), 1 deletion(-)
>> 
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index 0dc219dbcf..4facc76953 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
>> @@ -46,6 +46,7 @@ extern char **environ;
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  
>>  #ifdef FIFREEZE
>>  #define CONFIG_FSFREEZE
>> @@ -1072,6 +1073,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
>> FsMount *mount,
>> Error **errp)
>>  {
>>  GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
>> +struct statvfs buf;
>> +unsigned long used, nonroot_total;
>> +double usage;
>>  char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
>>  mount->devmajor, mount->devminor);
>>  
>> @@ -1079,7 +1083,20 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
>> FsMount *mount,
>>  fs->type = g_strdup(mount->devtype);
>>  build_guest_fsinfo_for_device(devpath, fs, errp);
>>  
>> +if (statvfs(fs->mountpoint, )) {
>> +fs->has_usage = false;
>> +fs->usage = -1;
>> +} else {
>> +used = buf.f_blocks - buf.f_bfree;
>> +nonroot_total = used + buf.f_bavail;
>> +usage = (double) used / nonroot_total;
>
>Why calculate the usage here ?  IMHO it would make the command more useful
>if we just reported two separate "used_bytes" and "total_bytes" values. The
>app can convert to a percentage utilization value if they so desire, while
>other apps can now get the raw values

Thanks for your advice.
Will be fixed in v5.
Also adding support for qga-win.

Regards,
- Chen


[Qemu-devel] [PATCH v5 1/2] qga: add mountpoint usage to GuestFilesystemInfo

2018-06-01 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch adds support for getting the usage of mounted
filesystem.
The usage of fs stored as used_bytes and total_bytes.
It's very useful when we try to monitor guest's filesystem.

Cc: Michael Roth 
Cc: Eric Blake 
Cc: Daniel P. Berrangé 
Signed-off-by: Chen Hanxiao 

---
v2:
   add description in qapi-schema and version numbers
v3:
   use float for usage to get more precision.
v4:
   make usage a best-effort query and mark it as optional.
v5:
   report used-bytes and total-bytes in usage

 qga/commands-posix.c | 19 +++
 qga/qapi-schema.json | 14 +-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0dc219dbcf..83192f284c 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -46,6 +46,7 @@ extern char **environ;
 #include 
 #include 
 #include 
+#include 
 
 #ifdef FIFREEZE
 #define CONFIG_FSFREEZE
@@ -1072,6 +1073,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
Error **errp)
 {
 GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
+GuestFsUsage *usage;
+struct statvfs buf;
+unsigned long used, nonroot_total, fr_size;
 char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
 mount->devmajor, mount->devminor);
 
@@ -1079,7 +1083,22 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
 fs->type = g_strdup(mount->devtype);
 build_guest_fsinfo_for_device(devpath, fs, errp);
 
+if (statvfs(fs->mountpoint, )) {
+fs->has_usage = false;
+} else {
+fr_size = buf.f_frsize;
+used = buf.f_blocks - buf.f_bfree;
+nonroot_total = used + buf.f_bavail;
+usage = g_malloc0(sizeof(*usage));
+usage->used_bytes = used * fr_size;
+usage->total_bytes = nonroot_total * fr_size;
+
+fs->has_usage = true;
+fs->usage = usage;
+}
+
 g_free(devpath);
+
 return fs;
 }
 
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..2f742474f6 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -840,12 +840,24 @@
'bus-type': 'GuestDiskBusType',
'bus': 'int', 'target': 'int', 'unit': 'int'} }
 
+##
+# @GuestFsUsage:
+#
+# @used-bytes:  file system used bytes
+# @total-bytes: file system total bytes for nonroot user
+#
+# Since: 3.0
+##
+{ 'struct': 'GuestFsUsage',
+  'data': {'used-bytes': 'uint64', 'total-bytes': 'uint64'} }
+
 ##
 # @GuestFilesystemInfo:
 #
 # @name: disk name
 # @mountpoint: mount point path
 # @type: file system type string
+# @usage: file system usage struct (since 3.0)
 # @disk: an array of disk hardware information that the volume lies on,
 #which may be empty if the disk type is not supported
 #
@@ -853,7 +865,7 @@
 ##
 { 'struct': 'GuestFilesystemInfo',
   'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
-   'disk': ['GuestDiskAddress']} }
+   '*usage': 'GuestFsUsage', 'disk': ['GuestDiskAddress']} }
 
 ##
 # @guest-get-fsinfo:
-- 
2.17.0




[Qemu-devel] [PATCH v5 0/2] qga: report the usage of fs in

2018-06-01 Thread Chen Hanxiao
This series report the usage of guest's fs
by "used_bytes" and "total_bytes" by guest-get-fsinfo.

Chen Hanxiao (2):
  qga: add mountpoint usage to GuestFilesystemInfo
  qga-win: add  driver path usage to GuestFilesystemInfo

 qga/commands-posix.c | 19 +++
 qga/commands-win32.c | 13 +
 qga/qapi-schema.json | 14 +-
 3 files changed, 45 insertions(+), 1 deletion(-)

-- 
2.17.0




[Qemu-devel] [PATCH v5 2/2] qga-win: add driver path usage to GuestFilesystemInfo

2018-06-01 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch adds support for getting the usage of
windows driver path.
The usage of fs stored as used_bytes and total_bytes.

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
 qga/commands-win32.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/qga/commands-win32.c b/qga/commands-win32.c
index 2d48394748..badcc2e2e8 100644
--- a/qga/commands-win32.c
+++ b/qga/commands-win32.c
@@ -670,7 +670,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(char *guid, 
Error **errp)
 char fs_name[32];
 char vol_info[MAX_PATH+1];
 size_t len;
+uint64_t i64FreeBytesToCaller, i64TotalBytes, i64FreeBytes;
 GuestFilesystemInfo *fs = NULL;
+GuestFsUsage *usage = NULL;
 
 GetVolumePathNamesForVolumeName(guid, (LPCH), 0, _size);
 if (GetLastError() != ERROR_MORE_DATA) {
@@ -699,10 +701,21 @@ static GuestFilesystemInfo *build_guest_fsinfo(char 
*guid, Error **errp)
 fs_name[sizeof(fs_name) - 1] = 0;
 fs = g_malloc(sizeof(*fs));
 fs->name = g_strdup(guid);
+fs->has_usage = false;
 if (len == 0) {
 fs->mountpoint = g_strdup("System Reserved");
 } else {
 fs->mountpoint = g_strndup(mnt_point, len);
+if (GetDiskFreeSpaceEx(fs->mountpoint,
+   (PULARGE_INTEGER) & i64FreeBytesToCaller,
+   (PULARGE_INTEGER) & i64TotalBytes,
+   (PULARGE_INTEGER) & i64FreeBytes)) {
+usage = g_malloc(sizeof(*usage));
+usage->used_bytes = i64TotalBytes - i64FreeBytes;
+usage->total_bytes = i64TotalBytes;
+fs->usage = usage;
+fs->has_usage = true;
+}
 }
 fs->type = g_strdup(fs_name);
 fs->disk = build_guest_disk_info(guid, errp);
-- 
2.17.0




[Qemu-devel] [PATCH v4] qga: add mountpoint usage to GuestFilesystemInfo

2018-06-01 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch adds support for getting the usage of mounted
filesystem.
It's very useful when we try to monitor guest's filesystem.

Cc: Michael Roth 
Cc: Eric Blake 
Signed-off-by: Chen Hanxiao 

---
v2:
   add description in qapi-schema and version numbers
v3:
   use float for usage to get more precision.
v4:
   make usage as a best-effort query and mark it as optional.

 qga/commands-posix.c | 17 +
 qga/qapi-schema.json |  3 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0dc219dbcf..4facc76953 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -46,6 +46,7 @@ extern char **environ;
 #include 
 #include 
 #include 
+#include 
 
 #ifdef FIFREEZE
 #define CONFIG_FSFREEZE
@@ -1072,6 +1073,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
Error **errp)
 {
 GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
+struct statvfs buf;
+unsigned long used, nonroot_total;
+double usage;
 char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
 mount->devmajor, mount->devminor);
 
@@ -1079,7 +1083,20 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
 fs->type = g_strdup(mount->devtype);
 build_guest_fsinfo_for_device(devpath, fs, errp);
 
+if (statvfs(fs->mountpoint, )) {
+fs->has_usage = false;
+fs->usage = -1;
+} else {
+used = buf.f_blocks - buf.f_bfree;
+nonroot_total = used + buf.f_bavail;
+usage = (double) used / nonroot_total;
+
+fs->has_usage = true;
+fs->usage = usage;
+}
+
 g_free(devpath);
+
 return fs;
 }
 
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..fcd427e86d 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -846,6 +846,7 @@
 # @name: disk name
 # @mountpoint: mount point path
 # @type: file system type string
+# @usage: file system usage, fraction between 0 and 1 (since 3.0)
 # @disk: an array of disk hardware information that the volume lies on,
 #which may be empty if the disk type is not supported
 #
@@ -853,7 +854,7 @@
 ##
 { 'struct': 'GuestFilesystemInfo',
   'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
-   'disk': ['GuestDiskAddress']} }
+   '*usage': 'number', 'disk': ['GuestDiskAddress']} }
 
 ##
 # @guest-get-fsinfo:
-- 
2.17.0




[Qemu-devel] [PATCH v3] qga: add mountpoint usage to GuestFilesystemInfo

2018-05-30 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch adds support for getting the usage of mounted
filesystem.
It's very useful when we try to monitor guest's filesystem.

Cc: Michael Roth 
Cc: Eric Blake 

Signed-off-by: Chen Hanxiao 
---
v2:
   add description in qapi-schema and version numbers
v3:
   use float for usage to get more precision.

 qga/commands-posix.c | 16 
 qga/qapi-schema.json |  3 ++-
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0dc219dbcf..7601978a11 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -46,6 +46,7 @@ extern char **environ;
 #include 
 #include 
 #include 
+#include 
 
 #ifdef FIFREEZE
 #define CONFIG_FSFREEZE
@@ -1072,6 +1073,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
Error **errp)
 {
 GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
+struct statvfs buf;
+unsigned long used, nonroot_total;
+double usage;
 char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
 mount->devmajor, mount->devminor);
 
@@ -1079,7 +1083,19 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
 fs->type = g_strdup(mount->devtype);
 build_guest_fsinfo_for_device(devpath, fs, errp);
 
+if (statvfs(fs->mountpoint, )) {
+error_setg_errno(errp, errno, "Failed to get statvfs");
+return NULL;
+}
+
+used = buf.f_blocks - buf.f_bfree;
+nonroot_total = used + buf.f_bavail;
+usage = (double) used / nonroot_total;
+
+fs->usage = usage;
+
 g_free(devpath);
+
 return fs;
 }
 
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..68b9f60824 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -846,13 +846,14 @@
 # @name: disk name
 # @mountpoint: mount point path
 # @type: file system type string
+# @usage: file system usage, fraction between 0 and 1 (since 3.0)
 # @disk: an array of disk hardware information that the volume lies on,
 #which may be empty if the disk type is not supported
 #
 # Since: 2.2
 ##
 { 'struct': 'GuestFilesystemInfo',
-  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
+  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str', 'usage': 
'number',
'disk': ['GuestDiskAddress']} }
 
 ##
-- 
2.17.0




[Qemu-devel] [PATCH v2] qga: add mountpoint usage to GuestFilesystemInfo

2018-05-30 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch adds support for getting the usage of mounted
filesystem.
It's very useful when we try to monitor guest's filesystem.

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
v2:
   add description in qapi-schema and version numbers

 qga/commands-posix.c | 17 +
 qga/qapi-schema.json |  3 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0dc219dbcf..0d93c47a5d 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -46,6 +46,7 @@ extern char **environ;
 #include 
 #include 
 #include 
+#include 
 
 #ifdef FIFREEZE
 #define CONFIG_FSFREEZE
@@ -1072,6 +1073,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
Error **errp)
 {
 GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
+struct statvfs buf;
+unsigned long u100, used, nonroot_total;
+int usage;
 char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
 mount->devmajor, mount->devminor);
 
@@ -1079,7 +1083,20 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
 fs->type = g_strdup(mount->devtype);
 build_guest_fsinfo_for_device(devpath, fs, errp);
 
+if (statvfs(fs->mountpoint, )) {
+error_setg_errno(errp, errno, "Failed to get statvfs");
+return NULL;
+}
+
+used = buf.f_blocks - buf.f_bfree;
+u100 = 100 * used;
+nonroot_total = used + buf.f_bavail;
+usage = u100 / nonroot_total + (u100 % nonroot_total != 0);
+
+fs->usage = usage;
+
 g_free(devpath);
+
 return fs;
 }
 
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..98611b49af 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -846,13 +846,14 @@
 # @name: disk name
 # @mountpoint: mount point path
 # @type: file system type string
+# @usage: file system usage, integer between 0 and 100 (since 3.0)
 # @disk: an array of disk hardware information that the volume lies on,
 #which may be empty if the disk type is not supported
 #
 # Since: 2.2
 ##
 { 'struct': 'GuestFilesystemInfo',
-  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
+  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str', 'usage': 'int',
'disk': ['GuestDiskAddress']} }
 
 ##
-- 
2.17.0




Re: [Qemu-devel] [PATCH] qga: add mountpoint usage to GuestFilesystemInfo

2018-05-30 Thread Chen Hanxiao


At 2018-05-30 11:19:27, "Eric Blake"  wrote:
>On 05/29/2018 10:01 PM, Chen Hanxiao wrote:
>> From: Chen Hanxiao 
>> 
>> This patch adds support for getting the usage of mounted
>> filesystem.
>> It's very useful when we try to monitor guest's filesystem.
>> Use df of coreutils for reference.
>> 
>> Cc: Michael Roth 
>> Signed-off-by: Chen Hanxiao 
>> ---
>
>> @@ -1072,6 +1073,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
>> FsMount *mount,
>>  Error **errp)
>>   {
>>   GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
>> +struct statvfs buf;
>> +unsigned long u100, used, nonroot_total;
>> +int usage;
>>   char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
>>   mount->devmajor, mount->devminor);
>>   
>> @@ -1079,7 +1083,20 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
>> FsMount *mount,
>>   fs->type = g_strdup(mount->devtype);
>>   build_guest_fsinfo_for_device(devpath, fs, errp);
>>   
>> +if (statvfs(fs->mountpoint, )) {
>> +error_setg_errno(errp, errno, "Failed to get statvfs");
>> +return NULL;
>> +}
>> +
>> +used = buf.f_blocks - buf.f_bfree;
>> +u100 = 100 * used;
>> +nonroot_total = used + buf.f_bavail;
>> +usage = u100 / nonroot_total + (u100 % nonroot_total != 0);
>
>Why integral instead of floating point?

I followed the style of df from coreutils.
As the percentage already multiplied by 100,
I think it has enough precision.

>
>> +++ b/qga/qapi-schema.json
>> @@ -846,13 +846,14 @@
>>   # @name: disk name
>>   # @mountpoint: mount point path
>>   # @type: file system type string
>> +# @usage: file system usage
>
>Needs more details.  As written, it is an integer between 0 and 100; but 
>if you use floating point, a better description would be a fraction 
>between 0 and 1.

Will be updated in the following patch.

Regards,
- Chen

>
>>   # @disk: an array of disk hardware information that the volume lies on,
>>   #which may be empty if the disk type is not supported
>>   #
>>   # Since: 2.2
>>   ##
>>   { 'struct': 'GuestFilesystemInfo',
>> -  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
>> +  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str', 'usage': 
>> 'int',
>>  'disk': ['GuestDiskAddress']} }
>>   
>>   ##
>> 
>
>-- 
>Eric Blake, Principal Software Engineer
>Red Hat, Inc.   +1-919-301-3266
>Virtualization:  qemu.org | libvirt.org


[Qemu-devel] [PATCH] qga: add mountpoint usage to GuestFilesystemInfo

2018-05-29 Thread Chen Hanxiao
From: Chen Hanxiao 

This patch adds support for getting the usage of mounted
filesystem.
It's very useful when we try to monitor guest's filesystem.
Use df of coreutils for reference.

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 
---
 qga/commands-posix.c | 17 +
 qga/qapi-schema.json |  3 ++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0dc219dbcf..0d93c47a5d 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -46,6 +46,7 @@ extern char **environ;
 #include 
 #include 
 #include 
+#include 
 
 #ifdef FIFREEZE
 #define CONFIG_FSFREEZE
@@ -1072,6 +1073,9 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
Error **errp)
 {
 GuestFilesystemInfo *fs = g_malloc0(sizeof(*fs));
+struct statvfs buf;
+unsigned long u100, used, nonroot_total;
+int usage;
 char *devpath = g_strdup_printf("/sys/dev/block/%u:%u",
 mount->devmajor, mount->devminor);
 
@@ -1079,7 +1083,20 @@ static GuestFilesystemInfo *build_guest_fsinfo(struct 
FsMount *mount,
 fs->type = g_strdup(mount->devtype);
 build_guest_fsinfo_for_device(devpath, fs, errp);
 
+if (statvfs(fs->mountpoint, )) {
+error_setg_errno(errp, errno, "Failed to get statvfs");
+return NULL;
+}
+
+used = buf.f_blocks - buf.f_bfree;
+u100 = 100 * used;
+nonroot_total = used + buf.f_bavail;
+usage = u100 / nonroot_total + (u100 % nonroot_total != 0);
+
+fs->usage = usage;
+
 g_free(devpath);
+
 return fs;
 }
 
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..22c22d8a62 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -846,13 +846,14 @@
 # @name: disk name
 # @mountpoint: mount point path
 # @type: file system type string
+# @usage: file system usage
 # @disk: an array of disk hardware information that the volume lies on,
 #which may be empty if the disk type is not supported
 #
 # Since: 2.2
 ##
 { 'struct': 'GuestFilesystemInfo',
-  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str',
+  'data': {'name': 'str', 'mountpoint': 'str', 'type': 'str', 'usage': 'int',
'disk': ['GuestDiskAddress']} }
 
 ##
-- 
2.17.0




[Qemu-devel] [PATCH resend v2] qga: unset frozen state if no mount points are frozen

2018-05-29 Thread Chen Hanxiao
From: Chen Hanxiao 

If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
we may got nothing to freeze as all mountpoints are
not valid.
So call ga_unset_frozen in this senario.

Also, if we return 0 frozen fs, there is no need to call
guest-fsfreeze-thaw.

Cc: Michael Roth 
Signed-off-by: Chen Hanxiao 

---
v2:
  remove has_mountpoints special case
  add qapi-schema.json section

 qga/commands-posix.c | 6 ++
 qga/qapi-schema.json | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0dc219dbcf..93bfc5b4ab 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1274,6 +1274,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
has_mountpoints,
 }
 
 free_fs_mount_list();
+/* We may not issue any FIFREEZE here.
+ * Just unset ga_state here and ready for the next call.
+ */
+if (i == 0) {
+ga_unset_frozen(ga_state);
+}
 return i;
 
 error:
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..1045cef386 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -435,7 +435,9 @@
 # for up to 10 seconds by VSS.
 #
 # Returns: Number of file systems currently frozen. On error, all filesystems
-# will be thawed.
+# will be thawed. If no filesystems are frozen as a result of this call,
+# then @guest-fsfreeze-status will remain "thawed" and calling
+# @guest-fsfreeze-thaw is not necessary.
 #
 # Since: 0.15.0
 ##
-- 
2.14.3




Re: [Qemu-devel] [PATCH resend v2] qga: unset frozen state if no mount points are frozen

2018-04-21 Thread Chen Hanxiao

At 2018-04-02 14:15:48, "Chen Hanxiao" <chen_han_x...@126.com> wrote:
>From: Chen Hanxiao <chenhanx...@gmail.com>
>
>If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
>we may got nothing to freeze as all mountpoints are
>not valid.
>So call ga_unset_frozen in this senario.
>
>Also, if we return 0 frozen fs, there is no need to call
>guest-fsfreeze-thaw.
>
>Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
>Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
>

Any comments?

Regards,
- Chen

[Qemu-devel] [PATCH resend v2] qga: unset frozen state if no mount points are frozen

2018-04-02 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
we may got nothing to freeze as all mountpoints are
not valid.
So call ga_unset_frozen in this senario.

Also, if we return 0 frozen fs, there is no need to call
guest-fsfreeze-thaw.

Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>

---
v2:
  remove has_mountpoints special case
  add qapi-schema.json section

 qga/commands-posix.c | 6 ++
 qga/qapi-schema.json | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 0dc219dbcf..93bfc5b4ab 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1274,6 +1274,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
has_mountpoints,
 }
 
 free_fs_mount_list();
+/* We may not issue any FIFREEZE here.
+ * Just unset ga_state here and ready for the next call.
+ */
+if (i == 0) {
+ga_unset_frozen(ga_state);
+}
 return i;
 
 error:
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..1045cef386 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -435,7 +435,9 @@
 # for up to 10 seconds by VSS.
 #
 # Returns: Number of file systems currently frozen. On error, all filesystems
-# will be thawed.
+# will be thawed. If no filesystems are frozen as a result of this call,
+# then @guest-fsfreeze-status will remain "thawed" and calling
+# @guest-fsfreeze-thaw is not necessary.
 #
 # Since: 0.15.0
 ##
-- 
2.14.3




[Qemu-devel] [PATCH resend v2] qga: unset frozen state if no mount point is frozen

2018-03-14 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
we may got nothing to freeze as all mountpoints are
not valid.
So call ga_unset_frozen in this senario.

Also, if we return 0 frozen fs, there is no need to call
guest-fsfreeze-thaw.

Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
v2:
  remove has_mountpoints special case
  add qapi-schema.json section

 qga/commands-posix.c | 6 ++
 qga/qapi-schema.json | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 967061444a..05cf9caa04 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1274,6 +1274,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
has_mountpoints,
 }
 
 free_fs_mount_list();
+/* We may not issue any FIFREEZE here.
+ * Just unset ga_state here and ready for the next call.
+ */
+if (i == 0) {
+ga_unset_frozen(ga_state);
+}
 return i;
 
 error:
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..1045cef386 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -435,7 +435,9 @@
 # for up to 10 seconds by VSS.
 #
 # Returns: Number of file systems currently frozen. On error, all filesystems
-# will be thawed.
+# will be thawed. If no filesystems are frozen as a result of this call,
+# then @guest-fsfreeze-status will remain "thawed" and calling
+# @guest-fsfreeze-thaw is not necessary.
 #
 # Since: 0.15.0
 ##
-- 
2.14.3




[Qemu-devel] [PATCH v2] qga: unset frozen state if no mount point is frozen

2018-02-22 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
we may got nothing to freeze as all mountpoints are
not valid.
So call ga_unset_frozen in this senario.

Also, if we return 0 frozen fs, there is no need to call
guest-fsfreeze-thaw.

Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
v2:
  remove has_mountpoints special case
  add qapi-schema.json section

 qga/commands-posix.c | 6 ++
 qga/qapi-schema.json | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index 967061444a..05cf9caa04 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1274,6 +1274,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
has_mountpoints,
 }
 
 free_fs_mount_list();
+/* We may not issue any FIFREEZE here.
+ * Just unset ga_state here and ready for the next call.
+ */
+if (i == 0) {
+ga_unset_frozen(ga_state);
+}
 return i;
 
 error:
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 17884c7c70..1045cef386 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -435,7 +435,9 @@
 # for up to 10 seconds by VSS.
 #
 # Returns: Number of file systems currently frozen. On error, all filesystems
-# will be thawed.
+# will be thawed. If no filesystems are frozen as a result of this call,
+# then @guest-fsfreeze-status will remain "thawed" and calling
+# @guest-fsfreeze-thaw is not necessary.
 #
 # Since: 0.15.0
 ##
-- 
2.14.3




Re: [Qemu-devel] [Resend][PATCH] qga: unset frozen state if no mount points are frozen

2018-02-21 Thread Chen Hanxiao

At 2018-02-16 02:41:25, "Michael Roth" <mdr...@linux.vnet.ibm.com> wrote:
>Quoting Chen Hanxiao (2018-02-08 19:35:42)
>> From: Chen Hanxiao <chenhanx...@gmail.com>
>> 
>> If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
>> we may got nothing to freeze as all mountpoints are
>> not valid.
>> Call ga_unset_frozen in this senario.
>> 
>> Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
>> Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
>> ---
>> Rebase on master
>> 
>>  qga/commands-posix.c | 6 ++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index e809e382eb..9fd51f1d7a 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
>> @@ -1273,6 +1273,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
>> has_mountpoints,
>>  }
>> 
>>  free_fs_mount_list();
>> +/* We may not issue any FIFREEZE here when had mountpoints.
>> + * Just unset ga_state here and ready for the next call.
>> + */
>> +if (has_mountpoints && i == 0) {
>> +ga_unset_frozen(ga_state);
>> +}
>
>It seems odd to special-case has_mountpoints. Wouldn't:
>
>  if (i == 0) {
>...
>  }
>
>be more consistent? Then management could infer i==0 leaves qga in
>unfrozen state. Otherwise I'd rather just stick with expecting a
>gratuitous unfreeze() since it requires less special-casing on the
>management/ side.
>
>And if we do change this, we'd probably want to update
>qga/qapi-schema.json to reflect the behavior. I.e.:
>
>##
># @guest-fsfreeze-freeze:
>#
># Sync and freeze all freezable, local guest filesystems. If this
># command succeeded, you may call @guest-fsfreeze-thaw later to
># unfreeze.
>...
># Returns: Number of file systems currently frozen. On error, all filesystems
>-# will be thawed.
>+# will be thawed. If no filesystems are frozen as a result of this call,
>+# then @guest-fsfreeze-status will remain "thawed" and calling
>+# @guest-fsfreeze-thaw is not necessary.
>

Thanks for the review.
Will be fixed in v2.

Regards,
- Chen


[Qemu-devel] [Resend][PATCH] qga: unset frozen state if no mount points are frozen

2018-02-08 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
we may got nothing to freeze as all mountpoints are
not valid.
Call ga_unset_frozen in this senario.

Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
Rebase on master

 qga/commands-posix.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index e809e382eb..9fd51f1d7a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1273,6 +1273,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
has_mountpoints,
 }
 
 free_fs_mount_list();
+/* We may not issue any FIFREEZE here when had mountpoints.
+ * Just unset ga_state here and ready for the next call.
+ */
+if (has_mountpoints && i == 0) {
+ga_unset_frozen(ga_state);
+}
 return i;
 
 error:
-- 
2.14.3




[Qemu-devel] [resend][PATCH] qga: unset frozen state if no mount points are frozen

2018-01-11 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
we may got nothing to freeze as all mountpoints are
not valid.
Call ga_unset_frozen in this senario.

Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
Rebase on master

 qga/commands-posix.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index e809e382eb..9fd51f1d7a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1273,6 +1273,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
has_mountpoints,
 }
 
 free_fs_mount_list();
+/* We may not issue any FIFREEZE here when had mountpoints.
+ * Just unset ga_state here and ready for the next call.
+ */
+if (has_mountpoints && i == 0) {
+ga_unset_frozen(ga_state);
+}
 return i;
 
 error:
-- 
2.14.3




Re: [Qemu-devel] [PATCH] qga: unset frozen state if no mount points are frozen

2018-01-04 Thread Chen Hanxiao


At 2017-12-16 15:34:17, "Chen Hanxiao" <chen_han_x...@126.com> wrote:
>From: Chen Hanxiao <chenhanx...@gmail.com>
>
>If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
>we may got nothing to freeze as all mountpoints are
>not valid.
>Call ga_unset_frozen in this senario.
>
>Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
>Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
>---
> qga/commands-posix.c | 6 ++
> 1 file changed, 6 insertions(+)
>
>diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>index e809e382eb..9fd51f1d7a 100644
>--- a/qga/commands-posix.c
>+++ b/qga/commands-posix.c
>@@ -1273,6 +1273,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
>has_mountpoints,
> }
> 
> free_fs_mount_list();
>+/* We may not issue any FIFREEZE here when had mountpoints.
>+ * Just unset ga_state here and ready for the next call.
>+ */
>+if (has_mountpoints && i == 0) {
>+ga_unset_frozen(ga_state);
>+}
> return i;
> 
> error:
>-- 

Hi,
   Any comments?

Regards,
- Chen


[Qemu-devel] [PATCH] qga: unset frozen state if no mount points are frozen

2017-12-15 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
we may got nothing to freeze as all mountpoints are
not valid.
Call ga_unset_frozen in this senario.

Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 qga/commands-posix.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index e809e382eb..9fd51f1d7a 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -1273,6 +1273,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
has_mountpoints,
 }
 
 free_fs_mount_list();
+/* We may not issue any FIFREEZE here when had mountpoints.
+ * Just unset ga_state here and ready for the next call.
+ */
+if (has_mountpoints && i == 0) {
+ga_unset_frozen(ga_state);
+}
 return i;
 
 error:
-- 
2.14.3




Re: [Qemu-devel] [Resend][PATCH] qapi-docs: fix a comment typo

2017-11-07 Thread Chen Hanxiao

在 2017-11-08 03:43:20,"Michael Roth" <mdr...@linux.vnet.ibm.com> 写道:
>Quoting Michael Roth (2017-11-07 11:06:55)
>> Quoting Chen Hanxiao (2017-11-07 05:37:16)
>> > From: Chen Hanxiao <chenhanx...@gmail.com>
>> > 
>> >   s/Subection/Subsection
>> > 
>> > Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
>> > Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
>> 
>> Thanks, applied to qga tree:
>>   https://github.com/mdroth/qemu/commits/qga
>
>Actually looks like Markus picked this up yesterday, and I plucked it
>into the wrong tree anyway =\
>

Oops, I missed the mail from Markus.
Sorry for the noise.

Regards,
- Chen


[Qemu-devel] [Resend][PATCH] qapi-docs: fix a comment typo

2017-11-07 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

  s/Subection/Subsection

Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 docs/devel/qapi-code-gen.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index f04c63fe82..06ab699066 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -63,7 +63,7 @@ Comment text starting with '=' is a section title:
 
 Double the '=' for a subsection title:
 
-# == Subection title
+# == Subsection title
 
 '|' denotes examples:
 
-- 
2.13.5




Re: [Qemu-devel] [PATCH] qga-win: fall back to hardcoded user and group names if LookupAccountSidW failed

2017-11-02 Thread Chen Hanxiao

At 2017-10-27 09:05:25, "Michael Roth" <mdr...@linux.vnet.ibm.com> wrote:
>Quoting Tomáš Golembiovský (2017-10-26 08:54:37)
>> On Wed, 25 Oct 2017 16:58:07 -0500
>> Michael Roth <mdr...@linux.vnet.ibm.com> wrote:
>> 
>> > Quoting Tomáš Golembiovský (2017-09-29 16:31:02)
>> > > On Fri, 29 Sep 2017 17:11:22 +0800
>> > > Chen Hanxiao <chen_han_x...@126.com> wrote:
>> > > 
>> > > Reviewed-by: Tomáš Golembiovský <tgole...@redhat.com>  
>> > 
>> > I think it might be getNameByStringSID()/LookupAccountSidW() reporting a
>> > stale GetLastError() value.
>> > 
>> 
>> I suppose you're right! Taking a closer look there's one more issue with
>> getNameByStringSID(). The call ConvertStringSidToSidW() does not return
>> HRESULT so using chk() makes no sense.
>> 
>> I propose slightly modified version of your fix:
>> 
>> diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
>> index ba7c94eb25..65f68f94a3 100644
>> --- a/qga/vss-win32/install.cpp
>> +++ b/qga/vss-win32/install.cpp
>> @@ -148,10 +148,15 @@ static HRESULT getNameByStringSID(
>>  DWORD domainNameLen = BUFFER_SIZE;
>>  wchar_t domainName[BUFFER_SIZE];
>>  
>> -chk(ConvertStringSidToSidW(sid, ));
>> -LookupAccountSidW(NULL, psid, buffer, bufferLen,
>> -domainName, , );
>> -hr = HRESULT_FROM_WIN32(GetLastError());
>> +if (!ConvertStringSidToSidW(sid, ) {
>> +hr = HRESULT_FROM_WIN32(GetLastError());
>> +goto out;
>> +}
>> +if (!LookupAccountSidW(NULL, psid, buffer, bufferLen,
>> +   domainName, , )) {
>> +hr = HRESULT_FROM_WIN32(GetLastError());
>> +/* Fall through and free psid */
>> +}
>>  
>>  LocalFree(psid);
>
>
>Thanks! It's not yet clear if this fixes the issue completely (though it
>seems likely), but it's clearly a bug either way so I've gone ahead and
>applied it to the qga tree:
>
>  
> https://github.com/mdroth/qemu/commit/8cedc80555e5a395a4fda7130b0115015e775c48
>
>Chen, if there's still an issue with the updated patch please let me know.

Sorry for the late reply.

I tested the upstream qga-win on that windows VM for several times.
It works fine by my limit tests.

Regards,
- Chen



>
>>  
>> 
>> -- 
>> Tomáš Golembiovský <tgole...@redhat.com>
>> 
>


Re: [Qemu-devel] [PATCH] qga-win: fall back to hardcoded user and group names if LookupAccountSidW failed

2017-10-26 Thread Chen Hanxiao

At 2017-10-26 17:59:34, "Michael Roth" <mdr...@linux.vnet.ibm.com> wrote:
>Quoting Chen Hanxiao (2017-10-26 04:27:40)
>> 
>> 
>> At 2017-10-26 05:58:07, "Michael Roth" <mdr...@linux.vnet.ibm.com> wrote:
>> >Quoting Tomáš Golembiovský (2017-09-29 16:31:02)
>> >> On Fri, 29 Sep 2017 17:11:22 +0800
>> >> Chen Hanxiao <chen_han_x...@126.com> wrote:
>> >> 
>> >> > From: Chen Hanxiao <chenhanx...@gmail.com>
>> >> > 
>> >> > On some of windows (win08 sp2),
>> >> > it doesn't work by calling LookupAccountSidW with
>> >> > well-known SIDs,
>> >> > We got an error:
>> >> > error 997 overlapped I/O operation is in progress
>> >> > 
>> >> > But hardcoded names work.
>> >> > 
>> >> > This patch introduces a workaroud for this issue:
>> >> > if LookupAccountSidW failed, try hardcoded one.
>> >> > 
>> >> > Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
>> >> > ---
>> >> >  qga/vss-win32/install.cpp | 10 --
>> >> >  1 file changed, 8 insertions(+), 2 deletions(-)
>> >> > 
>> >> 
>> >> I wonder if it's caused by qga itself or a race/conflict with some other
>> >> app. But still...
>> >> 
>> >> 
>> >> Reviewed-by: Tomáš Golembiovský <tgole...@redhat.com>
>> >
>> >I think it might be getNameByStringSID()/LookupAccountSidW() reporting a
>> >stale GetLastError() value.
>> >
>> >Does this fix the issue?
>> 
>> Not exactly.
>> I tested your patch several times, it improved greatly.
>> But failed only one time,
>> got another error 1722.
>
>Hmm, was that error also from the getNameByStringSID() call?

I don't know how to trace qemu-ga-win,
but added some fprintf(stdout).

+hr = getNameByStringSID(administratorsGroupSID, buffer, );
+if (FAILED(hr)) {

I saw my logs inside if (FAILED(hr)) {

But it looks weird, as your patch already dealing with this senario.


Regards,
- Chen

>
>> 
>> Build with my fallback patch and your suggestion, 
>> installation work perfectly.
>
>Thanks for testing.
>
>> 


Re: [Qemu-devel] [PATCH] qga-win: fall back to hardcoded user and group names if LookupAccountSidW failed

2017-10-26 Thread Chen Hanxiao


At 2017-10-26 05:58:07, "Michael Roth" <mdr...@linux.vnet.ibm.com> wrote:
>Quoting Tomáš Golembiovský (2017-09-29 16:31:02)
>> On Fri, 29 Sep 2017 17:11:22 +0800
>> Chen Hanxiao <chen_han_x...@126.com> wrote:
>> 
>> > From: Chen Hanxiao <chenhanx...@gmail.com>
>> > 
>> > On some of windows (win08 sp2),
>> > it doesn't work by calling LookupAccountSidW with
>> > well-known SIDs,
>> > We got an error:
>> > error 997 overlapped I/O operation is in progress
>> > 
>> > But hardcoded names work.
>> > 
>> > This patch introduces a workaroud for this issue:
>> > if LookupAccountSidW failed, try hardcoded one.
>> > 
>> > Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
>> > ---
>> >  qga/vss-win32/install.cpp | 10 --
>> >  1 file changed, 8 insertions(+), 2 deletions(-)
>> > 
>> 
>> I wonder if it's caused by qga itself or a race/conflict with some other
>> app. But still...
>> 
>> 
>> Reviewed-by: Tomáš Golembiovský <tgole...@redhat.com>
>
>I think it might be getNameByStringSID()/LookupAccountSidW() reporting a
>stale GetLastError() value.
>
>Does this fix the issue?

Not exactly.
I tested your patch several times, it improved greatly.
But failed only one time,
got another error 1722.

Build with my fallback patch and your suggestion, 
installation work perfectly.

Regards,
- Chen

>
>diff --git a/qga/vss-win32/install.cpp b/qga/vss-win32/install.cpp
>index ba7c94eb25..94c921e8de 100644
>--- a/qga/vss-win32/install.cpp
>+++ b/qga/vss-win32/install.cpp
>@@ -149,9 +149,10 @@ static HRESULT getNameByStringSID(
> wchar_t domainName[BUFFER_SIZE];
> 
> chk(ConvertStringSidToSidW(sid, ));
>-LookupAccountSidW(NULL, psid, buffer, bufferLen,
>-domainName, , );
>-hr = HRESULT_FROM_WIN32(GetLastError());
>+if (!LookupAccountSidW(NULL, psid, buffer, bufferLen,
>+   domainName, , )) {
>+hr = HRESULT_FROM_WIN32(GetLastError());
>+}
> 
> LocalFree(psid);
>
>> 
>> 
>> -- 
>> Tomáš Golembiovský <tgole...@redhat.com>
>> 
>


[Qemu-devel] [Resend][PATCH] qga-win: don't hang if vss hold writes timeout

2017-10-17 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

When VM is in a heavy IO, if the command "guest-fsfreeze-freeze"
is executed, VSS may timeout when trying to hold writes.

Inside guest, Event ID 12298(VSS_ERROR_HOLD_WRITES_TIMEOUT)
is logged in the Event Viewer.

At that time, if we call AbortBackup, qga may hang forever.

This patch will solve this issue.

Cc: Michael Roth <mdr...@linux.vnet.ibm.com>
Cc: Tomoki Sekiyama <tomoki.sekiy...@gmail.com>

Reviewed-by: Tomoki Sekiyama <tomoki.sekiy...@gmail.com>
Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 qga/vss-win32/requester.cpp | 12 
 1 file changed, 12 insertions(+)

diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 301762d8b1..3d9c9716c0 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -419,6 +419,16 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
 break;
 }
 }
+
+if (wait_status == WAIT_TIMEOUT) {
+err_set(errset, E_FAIL,
+"timeout when try to receive Frozen event from VSS provider");
+/* If we are here, VSS had timeout.
+ * Don't call AbortBackup, just return directly.
+ */
+goto out1;
+}
+
 if (wait_status != WAIT_OBJECT_0) {
 err_set(errset, E_FAIL,
 "couldn't receive Frozen event from VSS provider");
@@ -432,6 +442,8 @@ out:
 if (vss_ctx.pVssbc) {
 vss_ctx.pVssbc->AbortBackup();
 }
+
+out1:
 requester_cleanup();
 CoUninitialize();
 }
-- 
2.13.5




[Qemu-devel] [PATCH] qapi-docs: fix a comment typo

2017-10-12 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

  s/Subection/Subsection

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 docs/devel/qapi-code-gen.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index f04c63fe82..06ab699066 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -63,7 +63,7 @@ Comment text starting with '=' is a section title:
 
 Double the '=' for a subsection title:
 
-# == Subection title
+# == Subsection title
 
 '|' denotes examples:
 
-- 
2.13.5




Re: [Qemu-devel] [PATCH] qga-win: don't hang if vss hold writes timeout

2017-09-26 Thread Chen Hanxiao

At 2017-09-22 10:46:06, "Chen Hanxiao" <chen_han_x...@126.com> wrote:
>From: Chen Hanxiao <chenhanx...@gmail.com>
>
>When VM is in a heavy IO, if the command "guest-fsfreeze-freeze"
>is executed, VSS may timeout when trying to hold writes.
>
>Inside guest, Event ID 12298(VSS_ERROR_HOLD_WRITES_TIMEOUT)
>is logged in the Event Viewer.
>
>At that time, if we call AbortBackup, qga may hang forever.
>
>This patch will solve this issues.
>
>Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>

Hi, Michael

 Any comments?

http://lists.nongnu.org/archive/html/qemu-devel/2017-09/msg06199.html

Regards,
- Chen

Re: [Qemu-devel] [PATCH] qga-win: don't hang if vss hold writes timeout

2017-09-22 Thread Chen Hanxiao

在 2017-09-23 05:12:20,"Tomoki Sekiyama" <tomoki.sekiy...@gmail.com> 写道:
 
> 
> Thanks for your patch.
> Which version of Windows have you found the hang?
> 
> 
  I tested on win08, win 08 r2 and win2012.

QGA versions:

  Use the latest qga-win from 
  
https://fedorapeople.org/groups/virt/virtio-win/direct-downloads/archive-qemu-ga/qemu-ga-win-7.4.5-1/
 
  and stable branch from
  
https://fedorapeople.org/groups/virt/virtio-win/direct-downloads/archive-virtio/virtio-win-0.1.126-2/virtio-win-0.1.126.iso

  All of them have the same behavior.

How to reproduce:

  Copy a big file in guest (from d: to c:), then do fsfreeze when IO is in 
progress.

  At this time, we had a very chance to see that qga hang for a long time.

  Libvirt will not recieve a reply.
  qemu-ga service even don't answer `net stop` command.


Regards,
- Chen
 
  

> Regards,
> Tomoki
> 
> 
> 2017-09-22 11:46 GMT+09:00 Chen Hanxiao <chen_han_x...@126.com>:
> 
> From: Chen Hanxiao <chenhanx...@gmail.com>
> 
> 
> 
> When VM is in a heavy IO, if the command "guest-fsfreeze-freeze"
> 
> is executed, VSS may timeout when trying to hold writes.
> 
> 
> 
> Inside guest, Event ID 12298(VSS_ERROR_HOLD_WRITES_TIMEOUT)
> 
> is logged in the Event Viewer.
> 
> 
> 
> At that time, if we call AbortBackup, qga may hang forever.
> 
> 
> 
> This patch will solve this issues.
> 
> 
> 
> Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
> 
> ---
> 
>  qga/vss-win32/requester.cpp | 12 
> 
>  1 file changed, 12 insertions(+)
> 
> 
> 
> diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
> 
> index 301762d..3d9c971 100644
> 
> --- a/qga/vss-win32/requester.cpp
> 
> +++ b/qga/vss-win32/requester.cpp
> 
> @@ -419,6 +419,16 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
> 
>              break;
> 
>          }
> 
>      }
> 
> +
> 
> +    if (wait_status == WAIT_TIMEOUT) {
> 
> +        err_set(errset, E_FAIL,
> 
> +                "timeout when try to receive Frozen event from VSS 
> provider");
> 
> +        /* If we are here, VSS had timeout.
> 
> +         * Don't call AbortBackup, just return directly.
> 
> +         */
> 
> +        goto out1;
> 
> +    }
> 
> +
> 
>      if (wait_status != WAIT_OBJECT_0) {
> 
>          err_set(errset, E_FAIL,
> 
>                  "couldn't receive Frozen event from VSS provider");
> 
> @@ -432,6 +442,8 @@ out:
> 
>      if (vss_ctx.pVssbc) {
> 
>          vss_ctx.pVssbc->AbortBackup();
> 
>      }
> 
> +
> 
> +out1:
> 
>      requester_cleanup();
> 
>      CoUninitialize();
> 
>  }





















[Qemu-devel] [PATCH] qga-win: don't hang if vss hold writes timeout

2017-09-21 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

When VM is in a heavy IO, if the command "guest-fsfreeze-freeze"
is executed, VSS may timeout when trying to hold writes.

Inside guest, Event ID 12298(VSS_ERROR_HOLD_WRITES_TIMEOUT)
is logged in the Event Viewer.

At that time, if we call AbortBackup, qga may hang forever.

This patch will solve this issues.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 qga/vss-win32/requester.cpp | 12 
 1 file changed, 12 insertions(+)

diff --git a/qga/vss-win32/requester.cpp b/qga/vss-win32/requester.cpp
index 301762d..3d9c971 100644
--- a/qga/vss-win32/requester.cpp
+++ b/qga/vss-win32/requester.cpp
@@ -419,6 +419,16 @@ void requester_freeze(int *num_vols, ErrorSet *errset)
 break;
 }
 }
+
+if (wait_status == WAIT_TIMEOUT) {
+err_set(errset, E_FAIL,
+"timeout when try to receive Frozen event from VSS provider");
+/* If we are here, VSS had timeout.
+ * Don't call AbortBackup, just return directly.
+ */
+goto out1;
+}
+
 if (wait_status != WAIT_OBJECT_0) {
 err_set(errset, E_FAIL,
 "couldn't receive Frozen event from VSS provider");
@@ -432,6 +442,8 @@ out:
 if (vss_ctx.pVssbc) {
 vss_ctx.pVssbc->AbortBackup();
 }
+
+out1:
 requester_cleanup();
 CoUninitialize();
 }
-- 
2.7.5




[Qemu-devel] [PATCH v2] vhost_net: don't enable vring if backend lack this feature

2016-09-23 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

If backend(such as dpdk) lack the feather,
don't assume it in vring_enable.
Or we may fail in vhost_net_start, then we can't use vhost.
This will bring compat issue with old version backend.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
v2: return 0 there if protocol features aren't negotiated.

 hw/net/vhost_net.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f2d49ad..a1b796a 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -425,11 +425,17 @@ int vhost_set_vring_enable(NetClientState *nc, int enable)
 {
 VHostNetState *net = get_vhost_net(nc);
 const VhostOps *vhost_ops = net->dev.vhost_ops;
-
-nc->vring_enable = enable;
+int ret;
 
 if (vhost_ops && vhost_ops->vhost_set_vring_enable) {
-return vhost_ops->vhost_set_vring_enable(>dev, enable);
+ret = vhost_ops->vhost_set_vring_enable(>dev, enable);
+/*
+ * Try to enable vring feature.
+ * If protocol features aren't negotiated, return 0 for back compat.
+ */
+if (ret == 0) {
+nc->vring_enable = enable;
+}
 }
 
 return 0;
-- 
1.8.3.1





[Qemu-devel] [PATCH resend] vhost_net: don't enable vring if backend lack this feature

2016-09-13 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

If backend(such as dpdk) lack this feature,
don't assume it and mark it in vring_enable.
Or we may fail in vhost_net_start,
then we can't use vhost net.
This will bring compat issue with old version backend.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 hw/net/vhost_net.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f2d49ad..3819e65 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -425,11 +425,17 @@ int vhost_set_vring_enable(NetClientState *nc, int enable)
 {
 VHostNetState *net = get_vhost_net(nc);
 const VhostOps *vhost_ops = net->dev.vhost_ops;
-
-nc->vring_enable = enable;
+int ret;
 
 if (vhost_ops && vhost_ops->vhost_set_vring_enable) {
-return vhost_ops->vhost_set_vring_enable(>dev, enable);
+ret = vhost_ops->vhost_set_vring_enable(>dev, enable);
+if (ret == 0) {
+nc->vring_enable = enable;
+} else {
+nc->vring_enable = false;
+}
+
+return ret;
 }
 
 return 0;
-- 
1.8.3.1





Re: [Qemu-devel] [PATCH] vhost_net: don't enable vring if backend lack this feature

2016-09-08 Thread Chen Hanxiao


At 2016-09-03 16:44:47, "Chen Hanxiao" <chen_han_x...@126.com> wrote:
>From: Chen Hanxiao <chenhanx...@gmail.com>
>
>If backend(such as dpdk) lack this feature,
>don't assume it and mark it in vring_enable.
>Or we may fail in vhost_net_start,
>then we can't use vhost net.
>This will bring compat issue with old version backend.
>
>Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
>---

Hi,

Any comments?

Regards,
- Chen

[Qemu-devel] [PATCH] vhost_net: don't enable vring if backend lack this feature

2016-09-03 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

If backend(such as dpdk) lack this feature,
don't assume it and mark it in vring_enable.
Or we may fail in vhost_net_start,
then we can't use vhost net.
This will bring compat issue with old version backend.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 hw/net/vhost_net.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index f2d49ad..3819e65 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -425,11 +425,17 @@ int vhost_set_vring_enable(NetClientState *nc, int enable)
 {
 VHostNetState *net = get_vhost_net(nc);
 const VhostOps *vhost_ops = net->dev.vhost_ops;
-
-nc->vring_enable = enable;
+int ret;
 
 if (vhost_ops && vhost_ops->vhost_set_vring_enable) {
-return vhost_ops->vhost_set_vring_enable(>dev, enable);
+ret = vhost_ops->vhost_set_vring_enable(>dev, enable);
+if (ret == 0) {
+nc->vring_enable = enable;
+} else {
+nc->vring_enable = false;
+}
+
+return ret;
 }
 
 return 0;
-- 
1.8.3.1





Re: [Qemu-devel] [PATCH] vhost-user: return if no net clients found

2016-09-01 Thread Chen Hanxiao

At 2016-09-01 20:52:44, "Marc-André Lureau" <marcandre.lur...@gmail.com> wrote:
>Hi
>
>On Thu, Sep 1, 2016 at 4:00 PM Chen Hanxiao <chen_han_x...@126.com> wrote:
>
>>
>> Hi, here is the backtrace:
>>
>> #0  net_vhost_user_event (opaque=0x7fc2f6893be0, event=5) at
>> net/vhost-user.c:196
>> #1  0x7fc2f4ebfb2b in tcp_chr_disconnect (chr=0x7fc2f68cc400) at
>> qemu-char.c:2837
>> #2  0x7fc2f4ebfba9 in tcp_chr_sync_read (chr=0x7fc2f68cc400,
>> buf=, len=) at qemu-char.c:2888
>> #3  0x7fc2f4ec106d in qemu_chr_fe_read_all (s=0x7fc2f68cc400,
>> buf=buf@entry=0x7fff5fda25b7 "", len=len@entry=1) at qemu-char.c:264
>> #4  0x7fc2f4f9a43a in net_vhost_user_watch (chan=,
>> cond=, opaque=) at net/vhost-user.c:190
>> #5  0x7fc2f321999a in g_main_context_dispatch () from
>> /lib64/libglib-2.0.so.0
>> #6  0x7fc2f4fd8fe8 in glib_pollfds_poll () at main-loop.c:209
>> #7  os_host_main_loop_wait (timeout=) at main-loop.c:254
>> #8  main_loop_wait (nonblocking=) at main-loop.c:503
>> #9  0x7fc2f4dd7b1e in main_loop () at vl.c:1818
>> #10 main (argc=, argv=, envp=> out>) at vl.c:4394
>>
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> net_vhost_user_event (opaque=0x7fc2f6893be0, event=5) at
>> net/vhost-user.c:207
>> 207trace_vhost_user_event(s->chr->label, event);
>>
>>
>thanks for the backtrace, that helps
>
>However, I fail to understand how that can happen, as there has to be at
>least one net_client to start qemu with vhost-user and that callback must
>have at least the first netclient still around because the opaque pointer
>is shared with the netclient struct. So it looks like something destroyed
>the netclient before the callback, and in this case, the opaque pointer is
>invalid, and things are going all wrong. But it can't be host-net-remove,
>since the net-client is not on a registered hub.

The call back give qemu_find_net_clients_except id == 'filename'.
But could not find a netclient match.
Then ncs[i] did not get a valid net client, then we will get a seg fault.

>
>Could you try to find a simple reproducer using qemu only?

I'll try.

Regards,
- Chen

>
>thanks
>
>-- 
>Marc-André Lureau


Re: [Qemu-devel] [PATCH] vhost-user: return if no net clients found

2016-09-01 Thread Chen Hanxiao

在 2016-09-01 19:43:31,"Marc-André Lureau" <marcandre.lur...@gmail.com> 写道:

Hi



On Thu, Sep 1, 2016 at 2:15 PM Chen Hanxiao <chen_han_x...@126.com> wrote:

From: Chen Hanxiao <chenhanx...@gmail.com>

If we can't find a suitable net client, return directly.
Or we will got a segmentation fault.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 net/vhost-user.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index b0595f8..fb96db7 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -210,6 +210,9 @@ static void net_vhost_user_event(void *opaque, int event)
   MAX_QUEUE_NUM);
 assert(queues < MAX_QUEUE_NUM);

+if (queues < 1)
+return;
+



qemu coding style has mandatory {} braces.


I don't understand what this patch fixes. even if queues == 0, there is not 
reason I can think of it would crash. Could you provide a backtrace?


A qemu-only reproducer would be really useful.


Hi, here is the backtrace:


#0  net_vhost_user_event (opaque=0x7fc2f6893be0, event=5) at 
net/vhost-user.c:196
#1  0x7fc2f4ebfb2b in tcp_chr_disconnect (chr=0x7fc2f68cc400) at 
qemu-char.c:2837
#2  0x7fc2f4ebfba9 in tcp_chr_sync_read (chr=0x7fc2f68cc400, buf=, len=) at qemu-char.c:2888
#3  0x7fc2f4ec106d in qemu_chr_fe_read_all (s=0x7fc2f68cc400, 
buf=buf@entry=0x7fff5fda25b7 "", len=len@entry=1) at qemu-char.c:264
#4  0x7fc2f4f9a43a in net_vhost_user_watch (chan=, 
cond=, opaque=) at net/vhost-user.c:190
#5  0x7fc2f321999a in g_main_context_dispatch () from 
/lib64/libglib-2.0.so.0
#6  0x7fc2f4fd8fe8 in glib_pollfds_poll () at main-loop.c:209
#7  os_host_main_loop_wait (timeout=) at main-loop.c:254
#8  main_loop_wait (nonblocking=) at main-loop.c:503
#9  0x7fc2f4dd7b1e in main_loop () at vl.c:1818
#10 main (argc=, argv=, envp=) at 
vl.c:4394


Program received signal SIGSEGV, Segmentation fault.
net_vhost_user_event (opaque=0x7fc2f6893be0, event=5) at net/vhost-user.c:207
207trace_vhost_user_event(s->chr->label, event);


Regards,
- Chen




[Qemu-devel] [PATCH] vhost-user: return if no net clients found

2016-09-01 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

If we can't find a suitable net client, return directly.
Or we will got a segmentation fault.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 net/vhost-user.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/vhost-user.c b/net/vhost-user.c
index b0595f8..fb96db7 100644
--- a/net/vhost-user.c
+++ b/net/vhost-user.c
@@ -210,6 +210,9 @@ static void net_vhost_user_event(void *opaque, int event)
   MAX_QUEUE_NUM);
 assert(queues < MAX_QUEUE_NUM);
 
+if (queues < 1)
+return;
+
 s = DO_UPCAST(VhostUserState, nc, ncs[0]);
 trace_vhost_user_event(s->chr->label, event);
 switch (event) {
-- 
1.8.3.1





Re: [Qemu-devel] [PATCH v2 1/2] qmp: add support for system_suspend

2015-12-01 Thread Chen Hanxiao


On 12/01/2015 02:10 AM, Eric Blake wrote:

On 11/27/2015 08:01 PM, Chen Hanxiao wrote:

From: Chen Hanxiao <chenhanx...@gmail.com>

This patch add support for system_suspend qmp command.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
  qapi-schema.json |  9 +
  qmp-commands.hx  | 21 +
  qmp.c|  5 +
  3 files changed, 35 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 8b1a423..78bbb29 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3971,3 +3971,12 @@
  ##
  { 'enum': 'ReplayMode',
'data': [ 'none', 'record', 'play' ] }
+
+##
+# @system_suspend:
+#
+# Performs suspend operation of a guest.
+#
+# Since: 2.5
+##
+{ 'command': 'system_suspend' }

You've missed 2.5; this should be since 2.6.  Also, new QMP commands
should be named with '-' rather than '_'; so this should be
'system-suspend'.  (Yes, I know 'system_wakeup' already exists with the
older spelling).

How does this command differ from the existing ability to use
qemu-guest-agent to request the guest put itself into suspend state?


I tried to add a suspend hmp command which not existed.
As your previous comment, I added this for qmp.
It duplicates with current qga commands though.

So we just need that hmp patch with a qmp_system_suspend.

Regards,
- Chen




[Qemu-devel] [PATCH] qmp/hmp: add support for system_suspend

2015-11-27 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

This patch add support for system_suspend command.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 hmp-commands.hx | 14 ++
 hmp.c   |  5 +
 hmp.h   |  1 +
 qmp.c   |  5 +
 4 files changed, 25 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index bb52e4d..0ee9733 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -619,6 +619,20 @@ Power down the system (if supported).
 ETEXI
 
 {
+.name   = "system_suspend",
+.args_type  = "",
+.params = "",
+.help   = "send system suspend event",
+.mhandler.cmd = hmp_system_suspend,
+},
+
+STEXI
+@item system_suspend
+@findex system_suspend
+Suspend the system (if supported).
+ETEXI
+
+{
 .name   = "sum",
 .args_type  = "start:i,size:i",
 .params = "addr size",
diff --git a/hmp.c b/hmp.c
index 2140605..de4a5f7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -885,6 +885,11 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict)
 qmp_system_powerdown(NULL);
 }
 
+void hmp_system_suspend(Monitor *mon, const QDict *qdict)
+{
+qmp_system_suspend(NULL);
+}
+
 void hmp_cpu(Monitor *mon, const QDict *qdict)
 {
 int64_t cpu_index;
diff --git a/hmp.h b/hmp.h
index a8c5b5a..0064fa0 100644
--- a/hmp.h
+++ b/hmp.h
@@ -44,6 +44,7 @@ void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
 void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
+void hmp_system_suspend(Monitor *mon, const QDict *qdict);
 void hmp_cpu(Monitor *mon, const QDict *qdict);
 void hmp_memsave(Monitor *mon, const QDict *qdict);
 void hmp_pmemsave(Monitor *mon, const QDict *qdict);
diff --git a/qmp.c b/qmp.c
index 0a1fa19..408e418 100644
--- a/qmp.c
+++ b/qmp.c
@@ -119,6 +119,11 @@ void qmp_system_powerdown(Error **erp)
 qemu_system_powerdown_request();
 }
 
+void qmp_system_suspend(Error **erp)
+{
+qemu_system_suspend_request();
+}
+
 void qmp_cpu(int64_t index, Error **errp)
 {
 /* Just do nothing */
-- 
1.9.3





[Qemu-devel] [PATCH v2 1/2] qmp: add support for system_suspend

2015-11-27 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

This patch add support for system_suspend qmp command.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 qapi-schema.json |  9 +
 qmp-commands.hx  | 21 +
 qmp.c|  5 +
 3 files changed, 35 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 8b1a423..78bbb29 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3971,3 +3971,12 @@
 ##
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
+
+##
+# @system_suspend:
+#
+# Performs suspend operation of a guest.
+#
+# Since: 2.5
+##
+{ 'command': 'system_suspend' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 9d8b42f..4edb8bc 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -251,6 +251,27 @@ Example:
 EQMP
 
 {
+.name   = "system_suspend",
+.args_type  = "",
+.mhandler.cmd_new = qmp_marshal_system_suspend,
+},
+
+SQMP
+system_suspend
+
+
+Send system suspend event.
+
+Arguments: None.
+
+Example:
+
+-> { "execute": "system_suspend" }
+<- { "return": {} }
+
+EQMP
+
+{
 .name   = "system_powerdown",
 .args_type  = "",
 .mhandler.cmd_new = qmp_marshal_system_powerdown,
diff --git a/qmp.c b/qmp.c
index 0a1fa19..408e418 100644
--- a/qmp.c
+++ b/qmp.c
@@ -119,6 +119,11 @@ void qmp_system_powerdown(Error **erp)
 qemu_system_powerdown_request();
 }
 
+void qmp_system_suspend(Error **erp)
+{
+qemu_system_suspend_request();
+}
+
 void qmp_cpu(int64_t index, Error **errp)
 {
 /* Just do nothing */
-- 
1.9.3





[Qemu-devel] [PATCH v2 0/2] qmp/hmp: add support for system_suspend

2015-11-27 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

Chen Hanxiao (2):
  qmp: add support for system_suspend
  hmp: add support for system_suspend

 hmp-commands.hx  | 14 ++
 hmp.c|  5 +
 hmp.h|  1 +
 qapi-schema.json |  9 +
 qmp-commands.hx  | 21 +
 qmp.c|  5 +
 6 files changed, 55 insertions(+)

-- 
1.9.3





[Qemu-devel] [PATCH v2 2/2] hmp: add support for system_suspend

2015-11-27 Thread Chen Hanxiao
From: Chen Hanxiao <chenhanx...@gmail.com>

This patch add support for system_suspend hmp command.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
 hmp-commands.hx | 14 ++
 hmp.c   |  5 +
 hmp.h   |  1 +
 3 files changed, 20 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index bb52e4d..0ee9733 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -619,6 +619,20 @@ Power down the system (if supported).
 ETEXI
 
 {
+.name   = "system_suspend",
+.args_type  = "",
+.params = "",
+.help   = "send system suspend event",
+.mhandler.cmd = hmp_system_suspend,
+},
+
+STEXI
+@item system_suspend
+@findex system_suspend
+Suspend the system (if supported).
+ETEXI
+
+{
 .name   = "sum",
 .args_type  = "start:i,size:i",
 .params = "addr size",
diff --git a/hmp.c b/hmp.c
index 2140605..de4a5f7 100644
--- a/hmp.c
+++ b/hmp.c
@@ -885,6 +885,11 @@ void hmp_system_powerdown(Monitor *mon, const QDict *qdict)
 qmp_system_powerdown(NULL);
 }
 
+void hmp_system_suspend(Monitor *mon, const QDict *qdict)
+{
+qmp_system_suspend(NULL);
+}
+
 void hmp_cpu(Monitor *mon, const QDict *qdict)
 {
 int64_t cpu_index;
diff --git a/hmp.h b/hmp.h
index a8c5b5a..0064fa0 100644
--- a/hmp.h
+++ b/hmp.h
@@ -44,6 +44,7 @@ void hmp_quit(Monitor *mon, const QDict *qdict);
 void hmp_stop(Monitor *mon, const QDict *qdict);
 void hmp_system_reset(Monitor *mon, const QDict *qdict);
 void hmp_system_powerdown(Monitor *mon, const QDict *qdict);
+void hmp_system_suspend(Monitor *mon, const QDict *qdict);
 void hmp_cpu(Monitor *mon, const QDict *qdict);
 void hmp_memsave(Monitor *mon, const QDict *qdict);
 void hmp_pmemsave(Monitor *mon, const QDict *qdict);
-- 
1.9.3





Re: [Qemu-devel] [PATCH] qmp/hmp: add support for system_suspend

2015-11-27 Thread Chen Hanxiao


On 11/28/2015 01:53 AM, Eric Blake wrote:

On 11/27/2015 08:15 AM, Chen Hanxiao wrote:

From: Chen Hanxiao <chenhanx...@gmail.com>

This patch add support for system_suspend command.

Signed-off-by: Chen Hanxiao <chenhanx...@gmail.com>
---
  hmp-commands.hx | 14 ++
  hmp.c   |  5 +
  hmp.h   |  1 +
  qmp.c   |  5 +
  4 files changed, 25 insertions(+)

Generally, adding a function in qmp.c is accompanied by an addition in
qapi-schema.json to expose the function via QMP.


Sure, I'll post v2 for this.

Regards,
- Chen




Re: [Qemu-devel] [PATCH v12 00/15] vfio-pci: pass the aer error to guest

2015-07-28 Thread Chen, Hanxiao
Hi, Alex

 -Original Message-
 From: Alex Williamson [mailto:alex.william...@redhat.com]
 To: Chen, Hanxiao
 Cc: qemu-devel@nongnu.org; Chen, Fan
 Subject: Re: [Qemu-devel] [PATCH v12 00/15] vfio-pci: pass the aer error to 
 guest
 
 On Thu, 2015-07-16 at 12:00 +0800, Chen Hanxiao wrote:
  From: Chen Fan chen.fan.f...@cn.fujitsu.com
 
  For now, when qemu receives an error from host aer report
  by vfio pci passthough devices, qemu just terminate the guest.
  Usually user want to know what error occurred
  rather than stop the guest.
 
  This patches add aer capability support for vfio device,
  then pass the error to guest, and let guest driver to recover
  from the error.
  Turning on SERR# for error forwording in bridge control register
  patch in seabios has been merged as commit 32ec3ee.
 
  notes:
this series patches enable aer support single/multi-function,
for multi-function, require all the function of the slot assigned to
VM and on the same slot.
 
  Chen Fan (15):
vfio: extract vfio_get_hot_reset_info as a single function
vfio: squeeze out vfio_pci_do_hot_reset for support bus reset
pcie: modify the capability size assert
vfio: make the 4 bytes aligned for capability size
vfio: add pcie extanded capability support
aer: impove pcie_aer_init to support vfio device
vfio: add aer support for vfio device
vfio: add check host bus reset is support or not
pci: add bus reset_notifiers callbacks for host bus reset
vfio: add sec_bus_reset notifier to notify physical bus reset is
  needed
vfio: modify vfio_pci_hot_reset to support bus reset
vfio: do hot bus reset when do virtual secondary bus reset
pcie_aer: expose pcie_aer_msg() interface
vfio-pci: pass the aer error to guest
vfio: add 'aer' property to expose aercap
 
   hw/pci-bridge/ioh3420.c|   2 +-
   hw/pci-bridge/xio3130_downstream.c |   2 +-
   hw/pci-bridge/xio3130_upstream.c   |   2 +-
   hw/pci/pci.c   |  16 +
   hw/pci/pci_bridge.c|   6 +
   hw/pci/pcie.c  |   2 +-
   hw/pci/pcie_aer.c  |   6 +-
   hw/vfio/pci.c  | 636 
  +
   include/hw/pci/pci.h   |   4 +
   include/hw/pci/pci_bus.h   |   2 +
   include/hw/pci/pcie_aer.h  |   3 +-
   11 files changed, 609 insertions(+), 72 deletions(-)
 
 
 This seems to be pretty much the same as v11 where I commented that I
 didn't think it was acceptable to have a feature dependent on having all
 the functions assigned without supporting hot-add of multi-function
 devices.  Can you summarize what's changed here and whether that comment
 was addressed.  It would be a courtesy to reviewers to provide at least
 a summary changelog with each new version.  Thanks,
 

We could hot-unplug all passthrough devices by device_del,
But currently Qemu could not hot-add multi-function pci device.

See TODO in pcie_cap_slot_hotplug_cb:
/* TODO: multifunction hot-plug.
 * Right now, only a device of function = 0 is allowed to be
 * hot plugged/unplugged.
 */
assert(PCI_FUNC(pci_dev-devfn) == 0);

So we had to limit this as a workaround.

Why can't  we add functions one by one to the same slot by device_add?

If we could add functions one by one,
then we can enable aer for the devices once all dependence functions
were added by setting aer as a dynamic property(such as using 
object_property_add, qom-set)

How do you think?

Regards,
- Chen


Re: [Qemu-devel] [PATCH v2] exec: use macro ROUND_UP for alignment

2015-07-28 Thread Chen, Hanxiao

 -Original Message-
 From: qemu-devel-bounces+chenhanxiao=cn.fujitsu@nongnu.org
 [mailto:qemu-devel-bounces+chenhanxiao=cn.fujitsu@nongnu.org] On Behalf Of
 Chen Hanxiao
 Sent: Friday, July 24, 2015 11:12 AM
 To: Paolo Bonzini
 Cc: qemu-devel@nongnu.org
 Subject: [Qemu-devel] [PATCH v2] exec: use macro ROUND_UP for alignment
 
 Use ROUND_UP instead.
 
 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
  exec.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/exec.c b/exec.c
 index 7d60e15..bb16c50 100644
 --- a/exec.c
 +++ b/exec.c
 @@ -1207,7 +1207,7 @@ static void *file_ram_alloc(RAMBlock *block,
  unlink(filename);
  g_free(filename);
 
 -memory = (memory+hpagesize-1)  ~(hpagesize-1);
 +memory = ROUND_UP(memory, hpagesize);
 
  /*
   * ftruncate is not supported by hugetlbfs in older
 --
 2.1.0
 

ping

Regards,
- Chen



[Qemu-devel] [PATCH v2] exec: use macro ROUND_UP for alignment

2015-07-23 Thread Chen Hanxiao
Use ROUND_UP instead.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 exec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index 7d60e15..bb16c50 100644
--- a/exec.c
+++ b/exec.c
@@ -1207,7 +1207,7 @@ static void *file_ram_alloc(RAMBlock *block,
 unlink(filename);
 g_free(filename);
 
-memory = (memory+hpagesize-1)  ~(hpagesize-1);
+memory = ROUND_UP(memory, hpagesize);
 
 /*
  * ftruncate is not supported by hugetlbfs in older
-- 
2.1.0




[Qemu-devel] [PATCH] exec: use macro for alignment

2015-07-23 Thread Chen Hanxiao
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 exec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index 7d60e15..f84b485 100644
--- a/exec.c
+++ b/exec.c
@@ -1153,6 +1153,8 @@ static long gethugepagesize(const char *path, Error 
**errp)
 return fs.f_bsize;
 }
 
+#define ALIGN(x, y)  (((x)+(y)-1)  ~((y)-1))
+
 static void *file_ram_alloc(RAMBlock *block,
 ram_addr_t memory,
 const char *path,
@@ -1207,7 +1209,7 @@ static void *file_ram_alloc(RAMBlock *block,
 unlink(filename);
 g_free(filename);
 
-memory = (memory+hpagesize-1)  ~(hpagesize-1);
+memory = ALIGN(memory, hpagesize);
 
 /*
  * ftruncate is not supported by hugetlbfs in older
-- 
2.1.0




Re: [Qemu-devel] [PATCH v2] pci_add_capability: remove duplicate comments

2015-07-20 Thread Chen, Hanxiao


 -Original Message-
 From: qemu-devel-bounces+chenhanxiao=cn.fujitsu@nongnu.org
 [mailto:qemu-devel-bounces+chenhanxiao=cn.fujitsu@nongnu.org] On Behalf Of
 Chen Hanxiao
 Sent: Tuesday, July 14, 2015 4:16 PM
 To: Michael S. Tsirkin; qemu-devel@nongnu.org
 Subject: [Qemu-devel] [PATCH v2] pci_add_capability: remove duplicate comments
 
 Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
 ---
  hw/pci/pci.c | 6 ++
  1 file changed, 2 insertions(+), 4 deletions(-)
 
 diff --git a/hw/pci/pci.c b/hw/pci/pci.c
 index 442f822..a017614 100644
 --- a/hw/pci/pci.c
 +++ b/hw/pci/pci.c
 @@ -2101,12 +2101,10 @@ static void pci_del_option_rom(PCIDevice *pdev)
  }
 
  /*
 - * if !offset
 - * Reserve space and add capability to the linked list in pci config space
 - *
   * if offset = 0,
   * Find and reserve space and add capability to the linked list
 - * in pci config space */
 + * in pci config space
 + */
  int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
 uint8_t offset, uint8_t size)
  {
 --
 2.1.0
 

ping

Regards,
- Chen


[Qemu-devel] [PATCH v12 07/15] vfio: add aer support for vfio device

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

Calling pcie_aer_init to initilize aer related registers for
vfio device, then reload physical related registers to expose
device capability.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 89 +--
 1 file changed, 87 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index d7dc8a1..5e24e9d 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -32,6 +32,7 @@
 #include hw/pci/msi.h
 #include hw/pci/msix.h
 #include hw/pci/pci.h
+#include hw/pci/pci_bridge.h
 #include qemu-common.h
 #include qemu/error-report.h
 #include qemu/event_notifier.h
@@ -160,6 +161,8 @@ typedef struct VFIOPCIDevice {
 #define VFIO_FEATURE_ENABLE_VGA (1  VFIO_FEATURE_ENABLE_VGA_BIT)
 #define VFIO_FEATURE_ENABLE_REQ_BIT 1
 #define VFIO_FEATURE_ENABLE_REQ (1  VFIO_FEATURE_ENABLE_REQ_BIT)
+#define VFIO_FEATURE_ENABLE_AER_BIT 2
+#define VFIO_FEATURE_ENABLE_AER (1  VFIO_FEATURE_ENABLE_AER_BIT)
 int32_t bootindex;
 uint8_t pm_cap;
 bool has_vga;
@@ -2846,6 +2849,74 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t 
pos)
 return 0;
 }
 
+static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
+  int pos, uint16_t size)
+{
+PCIDevice *pdev = vdev-pdev;
+uint8_t *exp_cap = pdev-config + pdev-exp.exp_cap;
+PCIDevice *dev_iter;
+uint8_t type;
+uint32_t severity, errcap;
+int ret;
+
+if (!(vdev-features  VFIO_FEATURE_ENABLE_AER)) {
+pcie_add_capability(pdev, PCI_EXT_CAP_ID_ERR,
+cap_ver, pos, size);
+return 0;
+}
+
+dev_iter = pci_bridge_get_device(pdev-bus);
+if (!dev_iter) {
+goto error;
+}
+
+while (dev_iter) {
+type = pcie_cap_get_type(dev_iter);
+if ((type != PCI_EXP_TYPE_ROOT_PORT 
+ type != PCI_EXP_TYPE_UPSTREAM 
+ type != PCI_EXP_TYPE_DOWNSTREAM)) {
+goto error;
+}
+
+if (!dev_iter-exp.aer_cap) {
+goto error;
+}
+
+dev_iter = pci_bridge_get_device(dev_iter-bus);
+}
+
+errcap = vfio_pci_read_config(pdev, pdev-exp.aer_cap + PCI_ERR_CAP, 4);
+/*
+ * The ability to record multiple headers is depending on
+ * the state of the Multiple Header Recording Capable bit and
+ * enabled by the Multiple Header Recording Enable bit.
+ */
+if ((errcap  PCI_ERR_CAP_MHRC) 
+(errcap  PCI_ERR_CAP_MHRE)) {
+pdev-exp.aer_log.log_max = PCIE_AER_LOG_MAX_DEFAULT;
+} else {
+pdev-exp.aer_log.log_max = 0;
+}
+
+pcie_cap_deverr_init(pdev);
+ret = pcie_aer_init(pdev, pos, size);
+if (ret) {
+return ret;
+}
+
+/* load physical registers */
+severity = vfio_pci_read_config(pdev,
+   pdev-exp.aer_cap + PCI_ERR_UNCOR_SEVER, 4);
+pci_long_test_and_clear_mask(exp_cap + PCI_ERR_UNCOR_SEVER, ~severity);
+
+return 0;
+
+error:
+error_report(vfio: Unable to enable AER for device %s, parent bus 
+ does not support AER signaling, vdev-vbasedev.name);
+return -1;
+}
+
 static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
 {
 PCIDevice *pdev = vdev-pdev;
@@ -2853,6 +2924,7 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
 uint16_t cap_id, next, size;
 uint8_t cap_ver;
 uint8_t *config;
+int ret = 0;
 
 /*
  * In order to avoid breaking config space, create a copy to
@@ -2874,7 +2946,19 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
  */
 size = vfio_ext_cap_max_size(config, next);
 
-pcie_add_capability(pdev, cap_id, cap_ver, next, size);
+switch (cap_id) {
+case PCI_EXT_CAP_ID_ERR:
+ret = vfio_setup_aer(vdev, cap_ver, next, size);
+break;
+default:
+pcie_add_capability(pdev, cap_id, cap_ver, next, size);
+break;
+}
+
+if (ret) {
+goto out;
+}
+
 if (next == PCI_CONFIG_SPACE_SIZE) {
 /* Begin the rebuild, we should set the next offset zero. */
 pci_set_long(pdev-config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));
@@ -2884,8 +2968,9 @@ static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
 pci_set_long(vdev-emulated_config_bits + next, 0x);
 }
 
+out:
 g_free(config);
-return 0;
+return ret;
 }
 
 static int vfio_add_capabilities(VFIOPCIDevice *vdev)
-- 
2.1.0




[Qemu-devel] [PATCH v12 01/15] vfio: extract vfio_get_hot_reset_info as a single function

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

the function is used to get affected devices by bus reset.
so here extract it, and can used for aer soon.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 65 ++-
 1 file changed, 47 insertions(+), 18 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 2ed877f..b693ee7 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2669,6 +2669,50 @@ static void vfio_check_af_flr(VFIOPCIDevice *vdev, 
uint8_t pos)
 }
 }
 
+/*
+ * return negative with errno, return 0 on success.
+ * if success, the point of ret_info fill with the affected device reset info.
+ *
+ */
+static int vfio_get_hot_reset_info(VFIOPCIDevice *vdev,
+   struct vfio_pci_hot_reset_info **ret_info)
+{
+struct vfio_pci_hot_reset_info *info;
+int ret, count;
+
+*ret_info = NULL;
+
+info = g_malloc0(sizeof(*info));
+info-argsz = sizeof(*info);
+
+ret = ioctl(vdev-vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
+if (ret  errno != ENOSPC) {
+ret = -errno;
+goto error;
+}
+
+count = info-count;
+
+info = g_realloc(info, sizeof(*info) +
+ (count * sizeof(struct vfio_pci_dependent_device)));
+info-argsz = sizeof(*info) +
+  (count * sizeof(struct vfio_pci_dependent_device));
+
+ret = ioctl(vdev-vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
+if (ret) {
+ret = -errno;
+error_report(vfio: hot reset info failed: %m);
+goto error;
+}
+
+*ret_info = info;
+
+return 0;
+error:
+g_free(info);
+return ret;
+}
+
 static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
 {
 PCIDevice *pdev = vdev-pdev;
@@ -2808,7 +2852,7 @@ static bool vfio_pci_host_match(PCIHostDeviceAddress 
*host1,
 static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
 {
 VFIOGroup *group;
-struct vfio_pci_hot_reset_info *info;
+struct vfio_pci_hot_reset_info *info = NULL;
 struct vfio_pci_dependent_device *devices;
 struct vfio_pci_hot_reset *reset;
 int32_t *fds;
@@ -2820,12 +2864,8 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 vfio_pci_pre_reset(vdev);
 vdev-vbasedev.needs_reset = false;
 
-info = g_malloc0(sizeof(*info));
-info-argsz = sizeof(*info);
-
-ret = ioctl(vdev-vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
-if (ret  errno != ENOSPC) {
-ret = -errno;
+ret = vfio_get_hot_reset_info(vdev, info);
+if (ret) {
 if (!vdev-has_pm_reset) {
 error_report(vfio: Cannot reset device %04x:%02x:%02x.%x, 
  no available reset mechanism., vdev-host.domain,
@@ -2834,18 +2874,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 goto out_single;
 }
 
-count = info-count;
-info = g_realloc(info, sizeof(*info) + (count * sizeof(*devices)));
-info-argsz = sizeof(*info) + (count * sizeof(*devices));
 devices = info-devices[0];
-
-ret = ioctl(vdev-vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
-if (ret) {
-ret = -errno;
-error_report(vfio: hot reset info failed: %m);
-goto out_single;
-}
-
 trace_vfio_pci_hot_reset_has_dep_devices(vdev-vbasedev.name);
 
 /* Verify that we have all the groups required */
-- 
2.1.0




[Qemu-devel] [PATCH v12 15/15] vfio: add 'aer' property to expose aercap

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

add 'aer' property to let user able to decide whether expose
the aer capability. by default we should disable aer feature,
because it needs configuration restrictions.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 1e04c1e..eb69a39 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -4252,6 +4252,8 @@ static Property vfio_pci_dev_properties[] = {
 VFIO_FEATURE_ENABLE_VGA_BIT, false),
 DEFINE_PROP_BIT(x-req, VFIOPCIDevice, features,
 VFIO_FEATURE_ENABLE_REQ_BIT, true),
+DEFINE_PROP_BIT(aer, VFIOPCIDevice, features,
+VFIO_FEATURE_ENABLE_AER_BIT, false),
 DEFINE_PROP_INT32(bootindex, VFIOPCIDevice, bootindex, -1),
 DEFINE_PROP_BOOL(x-mmap, VFIOPCIDevice, vbasedev.allow_mmap, true),
 /*
-- 
2.1.0




[Qemu-devel] [PATCH v12 09/15] pci: add bus reset_notifiers callbacks for host bus reset

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

Particularly, For vfio devices, Once need to recovery devices
by bus reset such as AER, we always need to reset the host bus
to recovery the devices under the bus, so we need to add pci bus
callbacks to specify to do host bus reset.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/pci/pci.c | 16 
 hw/pci/pci_bridge.c  |  6 ++
 include/hw/pci/pci.h |  4 
 include/hw/pci/pci_bus.h |  2 ++
 4 files changed, 28 insertions(+)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 442f822..2784eee 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -75,11 +75,27 @@ static const VMStateDescription vmstate_pcibus = {
 }
 };
 
+void pci_bus_add_reset_notifier(PCIBus *bus, Notifier *notify)
+{
+notifier_list_add(bus-reset_notifiers, notify);
+}
+
+void pci_bus_remove_reset_notifier(Notifier *notify)
+{
+notifier_remove(notify);
+}
+
+void pci_bus_run_reset_notifier(PCIBus *bus, void *opaque)
+{
+notifier_list_notify(bus-reset_notifiers, opaque);
+}
+
 static void pci_bus_realize(BusState *qbus, Error **errp)
 {
 PCIBus *bus = PCI_BUS(qbus);
 
 vmstate_register(NULL, -1, vmstate_pcibus, bus);
+notifier_list_init(bus-reset_notifiers);
 }
 
 static void pci_bus_unrealize(BusState *qbus, Error **errp)
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 40c97b1..a8e3f57 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -267,6 +267,12 @@ void pci_bridge_write_config(PCIDevice *d,
 
 newctl = pci_get_word(d-config + PCI_BRIDGE_CONTROL);
 if (~oldctl  newctl  PCI_BRIDGE_CTL_BUS_RESET) {
+/*
+ * Notify all vfio-pci devices under the bus
+ * to do physical bus reset.
+ */
+pci_for_each_bus(s-sec_bus,
+pci_bus_run_reset_notifier, d);
 /* Trigger hot reset on 0-1 transition. */
 qbus_reset_all(s-sec_bus.qbus);
 }
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 551cb3d..fee04ff 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -7,6 +7,7 @@
 #include exec/memory.h
 #include sysemu/dma.h
 #include qapi/error.h
+#include qemu/notify.h
 
 /* PCI includes legacy ISA access.  */
 #include hw/isa/isa.h
@@ -381,6 +382,9 @@ void pci_bus_fire_intx_routing_notifier(PCIBus *bus);
 void pci_device_set_intx_routing_notifier(PCIDevice *dev,
   PCIINTxRoutingNotifier notifier);
 void pci_device_reset(PCIDevice *dev);
+void pci_bus_add_reset_notifier(PCIBus *bus, Notifier *notify);
+void pci_bus_remove_reset_notifier(Notifier *notify);
+void pci_bus_run_reset_notifier(PCIBus *bus, void *opaque);
 
 PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus,
const char *default_model,
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 403fec6..a529890 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -39,6 +39,8 @@ struct PCIBus {
Keep a count of the number of devices with raised IRQs.  */
 int nirq;
 int *irq_count;
+
+NotifierList reset_notifiers;
 };
 
 typedef struct PCIBridgeWindows PCIBridgeWindows;
-- 
2.1.0




[Qemu-devel] [PATCH v12 00/15] vfio-pci: pass the aer error to guest

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

For now, when qemu receives an error from host aer report
by vfio pci passthough devices, qemu just terminate the guest.
Usually user want to know what error occurred
rather than stop the guest.

This patches add aer capability support for vfio device,
then pass the error to guest, and let guest driver to recover
from the error.
Turning on SERR# for error forwording in bridge control register
patch in seabios has been merged as commit 32ec3ee.

notes:
  this series patches enable aer support single/multi-function,
  for multi-function, require all the function of the slot assigned to
  VM and on the same slot.

Chen Fan (15):
  vfio: extract vfio_get_hot_reset_info as a single function
  vfio: squeeze out vfio_pci_do_hot_reset for support bus reset
  pcie: modify the capability size assert
  vfio: make the 4 bytes aligned for capability size
  vfio: add pcie extanded capability support
  aer: impove pcie_aer_init to support vfio device
  vfio: add aer support for vfio device
  vfio: add check host bus reset is support or not
  pci: add bus reset_notifiers callbacks for host bus reset
  vfio: add sec_bus_reset notifier to notify physical bus reset is
needed
  vfio: modify vfio_pci_hot_reset to support bus reset
  vfio: do hot bus reset when do virtual secondary bus reset
  pcie_aer: expose pcie_aer_msg() interface
  vfio-pci: pass the aer error to guest
  vfio: add 'aer' property to expose aercap

 hw/pci-bridge/ioh3420.c|   2 +-
 hw/pci-bridge/xio3130_downstream.c |   2 +-
 hw/pci-bridge/xio3130_upstream.c   |   2 +-
 hw/pci/pci.c   |  16 +
 hw/pci/pci_bridge.c|   6 +
 hw/pci/pcie.c  |   2 +-
 hw/pci/pcie_aer.c  |   6 +-
 hw/vfio/pci.c  | 636 +
 include/hw/pci/pci.h   |   4 +
 include/hw/pci/pci_bus.h   |   2 +
 include/hw/pci/pcie_aer.h  |   3 +-
 11 files changed, 609 insertions(+), 72 deletions(-)

-- 
2.1.0




[Qemu-devel] [PATCH v12 05/15] vfio: add pcie extanded capability support

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

For vfio pcie device, we could expose the extended capability on
PCIE bus. in order to avoid config space broken, we introduce
a copy config for parsing extended caps. and rebuild the pcie
extended config space.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 72 ++-
 1 file changed, 71 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index cae45a0..d7dc8a1 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2522,6 +2522,21 @@ static uint8_t vfio_std_cap_max_size(PCIDevice *pdev, 
uint8_t pos)
 return next - pos;
 }
 
+
+static uint16_t vfio_ext_cap_max_size(const uint8_t *config, uint16_t pos)
+{
+uint16_t tmp, next = PCIE_CONFIG_SPACE_SIZE;
+
+for (tmp = PCI_CONFIG_SPACE_SIZE; tmp;
+tmp = PCI_EXT_CAP_NEXT(pci_get_long(config + tmp))) {
+if (tmp  pos  tmp  next) {
+next = tmp;
+}
+}
+
+return next - pos;
+}
+
 static void vfio_set_word_bits(uint8_t *buf, uint16_t val, uint16_t mask)
 {
 pci_set_word(buf, (pci_get_word(buf)  ~mask) | val);
@@ -2831,16 +2846,71 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, 
uint8_t pos)
 return 0;
 }
 
+static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
+{
+PCIDevice *pdev = vdev-pdev;
+uint32_t header;
+uint16_t cap_id, next, size;
+uint8_t cap_ver;
+uint8_t *config;
+
+/*
+ * In order to avoid breaking config space, create a copy to
+ * use for parsing extended capabilities.
+ */
+config = g_memdup(pdev-config, vdev-config_size);
+
+for (next = PCI_CONFIG_SPACE_SIZE; next;
+ next = PCI_EXT_CAP_NEXT(pci_get_long(config + next))) {
+header = pci_get_long(config + next);
+cap_id = PCI_EXT_CAP_ID(header);
+cap_ver = PCI_EXT_CAP_VER(header);
+
+/*
+ * If it becomes important to configure extended capabilities to their
+ * actual size, use this as the default when it's something we don't
+ * recognize. Since QEMU doesn't actually handle many of the config
+ * accesses, exact size doesn't seem worthwhile.
+ */
+size = vfio_ext_cap_max_size(config, next);
+
+pcie_add_capability(pdev, cap_id, cap_ver, next, size);
+if (next == PCI_CONFIG_SPACE_SIZE) {
+/* Begin the rebuild, we should set the next offset zero. */
+pci_set_long(pdev-config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));
+}
+
+/* Use emulated header pointer to allow dropping extended caps */
+pci_set_long(vdev-emulated_config_bits + next, 0x);
+}
+
+g_free(config);
+return 0;
+}
+
 static int vfio_add_capabilities(VFIOPCIDevice *vdev)
 {
 PCIDevice *pdev = vdev-pdev;
+int ret;
 
 if (!(pdev-config[PCI_STATUS]  PCI_STATUS_CAP_LIST) ||
 !pdev-config[PCI_CAPABILITY_LIST]) {
 return 0; /* Nothing to add */
 }
 
-return vfio_add_std_cap(vdev, pdev-config[PCI_CAPABILITY_LIST]);
+ret = vfio_add_std_cap(vdev, pdev-config[PCI_CAPABILITY_LIST]);
+if (ret) {
+return ret;
+}
+
+/* on PCI bus, it doesn't make sense to expose extended capabilities. */
+if (!pci_is_express(pdev) ||
+!pci_bus_is_express(pdev-bus) ||
+!pci_get_long(pdev-config + PCI_CONFIG_SPACE_SIZE)) {
+return 0;
+}
+
+return vfio_add_ext_cap(vdev);
 }
 
 static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
-- 
2.1.0




[Qemu-devel] [PATCH v12 13/15] pcie_aer: expose pcie_aer_msg() interface

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

For vfio device, we need to propagate the aer error to
Guest OS. we use the pcie_aer_msg() to send aer error
to guest.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Reviewed-by: Marcel Apfelbaum mar...@redhat.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/pci/pcie_aer.c | 2 +-
 include/hw/pci/pcie_aer.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index 786f25d..7b287cf 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -370,7 +370,7 @@ static void pcie_aer_msg_root_port(PCIDevice *dev, const 
PCIEAERMsg *msg)
  *
  * Walk up the bus tree from the device, propagate the error message.
  */
-static void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
+void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg)
 {
 uint8_t type;
 
diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
index 156acb0..c2ee4e2 100644
--- a/include/hw/pci/pcie_aer.h
+++ b/include/hw/pci/pcie_aer.h
@@ -102,5 +102,6 @@ void pcie_aer_root_write_config(PCIDevice *dev,
 
 /* error injection */
 int pcie_aer_inject_error(PCIDevice *dev, const PCIEAERErr *err);
+void pcie_aer_msg(PCIDevice *dev, const PCIEAERMsg *msg);
 
 #endif /* QEMU_PCIE_AER_H */
-- 
2.1.0




[Qemu-devel] [PATCH v12 11/15] vfio: modify vfio_pci_hot_reset to support bus reset

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

the vfio_pci_hot_reset differentiate the single and multi in-used
devices for reset. but in multi case, when some dependent devices
are not assigned to VM, the devices can not be recovered by driver.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index bab72a4..c52456b 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3287,6 +3287,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 PCIHostDeviceAddress host;
 VFIOPCIDevice *tmp;
 VFIODevice *vbasedev_iter;
+bool found;
 
 host.domain = devices[i].segment;
 host.bus = devices[i].bus;
@@ -3316,6 +3317,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 goto out;
 }
 
+found = false;
 /* Prep dependent devices for reset and clear our marker. */
 QLIST_FOREACH(vbasedev_iter, group-device_list, next) {
 if (vbasedev_iter-type != VFIO_DEVICE_TYPE_PCI) {
@@ -3327,12 +3329,23 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 ret = -EINVAL;
 goto out_single;
 }
+found = true;
 vfio_pci_pre_reset(tmp);
 tmp-vbasedev.needs_reset = false;
 multi = true;
 break;
 }
 }
+
+/*
+ * If the dependent device is not assigned to VM, we should
+ * not call hot bus reset, otherwise, the dependent device
+ * can not be used in the future.
+ */
+if (!single  !found) {
+ret = -EINVAL;
+goto out_single;
+}
 }
 
 if (!single  !multi) {
-- 
2.1.0




[Qemu-devel] [PATCH v12 03/15] pcie: modify the capability size assert

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

 Device's Offset and size can reach PCIE_CONFIG_SPACE_SIZE,
 fix the corresponding assert.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Reviewed-by: Marcel Apfelbaum mar...@redhat.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/pci/pcie.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 6e28985..d66fb10 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -594,7 +594,7 @@ void pcie_add_capability(PCIDevice *dev,
 
 assert(offset = PCI_CONFIG_SPACE_SIZE);
 assert(offset  offset + size);
-assert(offset + size  PCIE_CONFIG_SPACE_SIZE);
+assert(offset + size = PCIE_CONFIG_SPACE_SIZE);
 assert(size = 8);
 assert(pci_is_express(dev));
 
-- 
2.1.0




[Qemu-devel] [PATCH v12 02/15] vfio: squeeze out vfio_pci_do_hot_reset for support bus reset

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

squeeze out vfio_pci_do_hot_reset to do host bus reset when AER recovery.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 75 +++
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index b693ee7..f9ff9c4 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2713,6 +2713,48 @@ error:
 return ret;
 }
 
+static int vfio_pci_do_hot_reset(VFIOPCIDevice *vdev,
+ struct vfio_pci_hot_reset_info *info)
+{
+VFIOGroup *group;
+struct vfio_pci_hot_reset *reset;
+int32_t *fds;
+int ret, i, count;
+struct vfio_pci_dependent_device *devices;
+
+/* Determine how many group fds need to be passed */
+count = 0;
+devices = info-devices[0];
+QLIST_FOREACH(group, vfio_group_list, next) {
+for (i = 0; i  info-count; i++) {
+if (group-groupid == devices[i].group_id) {
+count++;
+break;
+}
+}
+}
+
+reset = g_malloc0(sizeof(*reset) + (count * sizeof(*fds)));
+reset-argsz = sizeof(*reset) + (count * sizeof(*fds));
+fds = reset-group_fds[0];
+
+/* Fill in group fds */
+QLIST_FOREACH(group, vfio_group_list, next) {
+for (i = 0; i  info-count; i++) {
+if (group-groupid == devices[i].group_id) {
+fds[reset-count++] = group-fd;
+break;
+}
+}
+}
+
+/* Bus reset! */
+ret = ioctl(vdev-vbasedev.fd, VFIO_DEVICE_PCI_HOT_RESET, reset);
+g_free(reset);
+
+return ret;
+}
+
 static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
 {
 PCIDevice *pdev = vdev-pdev;
@@ -2854,9 +2896,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 VFIOGroup *group;
 struct vfio_pci_hot_reset_info *info = NULL;
 struct vfio_pci_dependent_device *devices;
-struct vfio_pci_hot_reset *reset;
-int32_t *fds;
-int ret, i, count;
+int ret, i;
 bool multi = false;
 
 trace_vfio_pci_hot_reset(vdev-vbasedev.name, single ? one : multi);
@@ -2935,34 +2975,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 goto out_single;
 }
 
-/* Determine how many group fds need to be passed */
-count = 0;
-QLIST_FOREACH(group, vfio_group_list, next) {
-for (i = 0; i  info-count; i++) {
-if (group-groupid == devices[i].group_id) {
-count++;
-break;
-}
-}
-}
-
-reset = g_malloc0(sizeof(*reset) + (count * sizeof(*fds)));
-reset-argsz = sizeof(*reset) + (count * sizeof(*fds));
-fds = reset-group_fds[0];
-
-/* Fill in group fds */
-QLIST_FOREACH(group, vfio_group_list, next) {
-for (i = 0; i  info-count; i++) {
-if (group-groupid == devices[i].group_id) {
-fds[reset-count++] = group-fd;
-break;
-}
-}
-}
-
-/* Bus reset! */
-ret = ioctl(vdev-vbasedev.fd, VFIO_DEVICE_PCI_HOT_RESET, reset);
-g_free(reset);
+ret = vfio_pci_do_hot_reset(vdev, info);
 
 trace_vfio_pci_hot_reset_result(vdev-vbasedev.name,
 ret ? %m : Success);
-- 
2.1.0




[Qemu-devel] [PATCH v12 04/15] vfio: make the 4 bytes aligned for capability size

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

this function search the capability from the end, the last
size should 0x100 - pos, not 0xff - pos.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index f9ff9c4..cae45a0 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2509,7 +2509,8 @@ static void vfio_unmap_bars(VFIOPCIDevice *vdev)
  */
 static uint8_t vfio_std_cap_max_size(PCIDevice *pdev, uint8_t pos)
 {
-uint8_t tmp, next = 0xff;
+uint8_t tmp;
+uint16_t next = PCI_CONFIG_SPACE_SIZE;
 
 for (tmp = pdev-config[PCI_CAPABILITY_LIST]; tmp;
  tmp = pdev-config[tmp + 1]) {
-- 
2.1.0




[Qemu-devel] [PATCH v12 12/15] vfio: do hot bus reset when do virtual secondary bus reset

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

when do virtual secondary bus reset, the vfio device under
this bus need to do host bus reset to reset the device.
so add this case.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 31 +++
 1 file changed, 31 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index c52456b..bd67608 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -4134,6 +4134,27 @@ static void vfio_exitfn(PCIDevice *pdev)
 vfio_unregister_bars(vdev);
 }
 
+static int vfio_pci_is_single_function(VFIOPCIDevice *vdev)
+{
+struct vfio_pci_hot_reset_info *info = NULL;
+int ret;
+
+ret = vfio_get_hot_reset_info(vdev, info);
+if (ret) {
+goto out;
+}
+
+if (info-count  1) {
+ret = 0;
+goto out;
+}
+
+ret = 1;
+out:
+g_free(info);
+return ret;
+}
+
 static void vfio_pci_reset(DeviceState *dev)
 {
 PCIDevice *pdev = DO_UPCAST(PCIDevice, qdev, dev);
@@ -4141,6 +4162,16 @@ static void vfio_pci_reset(DeviceState *dev)
 
 trace_vfio_pci_reset(vdev-vbasedev.name);
 
+if (vdev-needs_bus_reset) {
+vdev-needs_bus_reset = false;
+/* Avoid duplicate bus reset */
+if (vdev-vbasedev.needs_reset) {
+vfio_pci_hot_reset(vdev,
+vfio_pci_is_single_function(vdev) ? true : false);
+}
+return;
+}
+
 vfio_pci_pre_reset(vdev);
 
 if (vdev-resetfn  !vdev-resetfn(vdev)) {
-- 
2.1.0




[Qemu-devel] [PATCH v12 14/15] vfio-pci: pass the aer error to guest

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

when the vfio device encounters an uncorrectable error in host,
the vfio_pci driver will signal the eventfd registered by this
vfio device, the results in the qemu eventfd handler getting
invoked.

this patch is to pass the error to guest and have the guest driver
recover from the error.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 45 +++--
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index bd67608..1e04c1e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3581,18 +3581,51 @@ static void vfio_put_device(VFIOPCIDevice *vdev)
 static void vfio_err_notifier_handler(void *opaque)
 {
 VFIOPCIDevice *vdev = opaque;
+PCIDevice *dev = vdev-pdev;
+PCIEAERMsg msg = {
+.severity = 0,
+.source_id = (pci_bus_num(dev-bus)  8) | dev-devfn,
+};
 
 if (!event_notifier_test_and_clear(vdev-err_notifier)) {
 return;
 }
 
 /*
- * TBD. Retrieve the error details and decide what action
- * needs to be taken. One of the actions could be to pass
- * the error to the guest and have the guest driver recover
- * from the error. This requires that PCIe capabilities be
- * exposed to the guest. For now, we just terminate the
- * guest to contain the error.
+ * in case the real hardware configration has been changed,
+ * here we should recheck the bus reset capability.
+ */
+if ((vdev-features  VFIO_FEATURE_ENABLE_AER) 
+vfio_check_host_bus_reset(vdev)) {
+goto stop;
+}
+/*
+ * we should read the error details from the real hardware
+ * configuration spaces, here we only need to do is signaling
+ * to guest an uncorrectable error has occurred.
+ */
+if ((vdev-features  VFIO_FEATURE_ENABLE_AER) 
+dev-exp.aer_cap) {
+uint8_t *aer_cap = dev-config + dev-exp.aer_cap;
+uint32_t uncor_status;
+bool isfatal;
+
+uncor_status = vfio_pci_read_config(dev,
+   dev-exp.aer_cap + PCI_ERR_UNCOR_STATUS, 4);
+
+isfatal = uncor_status  pci_get_long(aer_cap + PCI_ERR_UNCOR_SEVER);
+
+msg.severity = isfatal ? PCI_ERR_ROOT_CMD_FATAL_EN :
+ PCI_ERR_ROOT_CMD_NONFATAL_EN;
+
+pcie_aer_msg(dev, msg);
+return;
+}
+
+stop:
+/*
+ * If the aer capability is not exposed to the guest. we just
+ * terminate the guest to contain the error.
  */
 
 error_report(%s(%04x:%02x:%02x.%x) Unrecoverable error detected.  
-- 
2.1.0




[Qemu-devel] [PATCH v12 08/15] vfio: add check host bus reset is support or not

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

when init vfio devices done, we should test all the devices supported
aer whether conflict with others. For each one, get the hot reset
info for the affected device list.  For each affected device, all
should attach to the VM and on the same slot. also, we should test
all of the non-AER supporting vfio-pci devices on or below the target
bus to verify they have a reset mechanism.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 162 +++---
 1 file changed, 155 insertions(+), 7 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 5e24e9d..ed4d87e 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -32,6 +32,7 @@
 #include hw/pci/msi.h
 #include hw/pci/msix.h
 #include hw/pci/pci.h
+#include hw/pci/pci_bus.h
 #include hw/pci/pci_bridge.h
 #include qemu-common.h
 #include qemu/error-report.h
@@ -2849,6 +2850,133 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, 
uint8_t pos)
 return 0;
 }
 
+static bool vfio_pci_host_match(PCIHostDeviceAddress *host1,
+PCIHostDeviceAddress *host2)
+{
+return (host1-domain == host2-domain  host1-bus == host2-bus 
+host1-slot == host2-slot  host1-function == host2-function);
+}
+
+static int vfio_check_host_bus_reset(VFIOPCIDevice *vdev)
+{
+PCIDevice *pdev = vdev-pdev;
+struct vfio_pci_hot_reset_info *info = NULL;
+struct vfio_pci_dependent_device *devices;
+VFIOGroup *group;
+int ret, i, k;
+
+ret = vfio_get_hot_reset_info(vdev, info);
+if (ret) {
+error_report(vfio: Cannot get hot reset info);
+return ret;
+}
+
+/* List all affected devices by bus reset */
+devices = info-devices[0];
+
+/* Verify that we have all the groups required */
+for (i = 0; i  info-count; i++) {
+PCIHostDeviceAddress host;
+VFIOPCIDevice *tmp;
+VFIODevice *vbasedev;
+bool found = false;
+
+host.domain = devices[i].segment;
+host.bus = devices[i].bus;
+host.slot = PCI_SLOT(devices[i].devfn);
+host.function = PCI_FUNC(devices[i].devfn);
+
+/* Skip the current device */
+if (vfio_pci_host_match(host, vdev-host)) {
+continue;
+}
+
+/* Ensure we own the group of the affected device */
+QLIST_FOREACH(group, vfio_group_list, next) {
+if (group-groupid == devices[i].group_id) {
+break;
+}
+}
+
+if (!group) {
+/* Print all the device associated with that
+ * unfound group_id in vfio_group_list.
+ */
+for (k = 0; k info-count; k++) {
+if (devices[k].group_id == devices[i].group_id)
+error_report(vfio: Cannot enable AER for device %s 
+   depends on group %d which is not owned.,
+   vdev-vbasedev.name,
+   devices[k].group_id);
+}
+
+ret = -1;
+goto out;
+}
+
+/* Ensure affected devices for reset in the same slot */
+QLIST_FOREACH(vbasedev, group-device_list, next) {
+if (vbasedev-type != VFIO_DEVICE_TYPE_PCI) {
+continue;
+}
+tmp = container_of(vbasedev, VFIOPCIDevice, vbasedev);
+if (vfio_pci_host_match(host, tmp-host)) {
+PCIDevice *pci = PCI_DEVICE(tmp);
+
+if (pci-bus == pdev-bus 
+PCI_SLOT(pci-devfn) == PCI_SLOT(pdev-devfn)) {
+found = true;
+break;
+}
+error_report(vfio: Cannot enable AER for device %s, 
+ the dependent device %s is not in the same slot,
+ vdev-vbasedev.name, tmp-vbasedev.name);
+ret = -1;
+goto out;
+}
+}
+
+/* Ensure all affected devices assigned to VM */
+if (!found) {
+error_report(vfio: Cannot enable AER for device %s, 
+ the dependent device %04x:%02x:%02x.%x 
+ is not assigned to VM.,
+ vdev-vbasedev.name, host.domain, host.bus,
+ host.slot, host.function);
+ret = -1;
+goto out;
+}
+}
+
+ret = 0;
+out:
+g_free(info);
+return ret;
+}
+
+static int vfio_check_devices_host_bus_reset(void)
+{
+VFIOGroup *group;
+VFIODevice *vbasedev;
+VFIOPCIDevice *vdev;
+
+/* Check whether all of vfio-pci devices have bus reset capability */
+QLIST_FOREACH(group, vfio_group_list, next) {
+QLIST_FOREACH(vbasedev, group-device_list, next) {
+if (vbasedev-type != VFIO_DEVICE_TYPE_PCI) {
+continue

[Qemu-devel] [PATCH v12 10/15] vfio: add sec_bus_reset notifier to notify physical bus reset is needed

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/vfio/pci.c | 83 +++
 1 file changed, 83 insertions(+)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index ed4d87e..bab72a4 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -156,6 +156,7 @@ typedef struct VFIOPCIDevice {
 PCIHostDeviceAddress host;
 EventNotifier err_notifier;
 EventNotifier req_notifier;
+Notifier sec_bus_reset_notifier;
 int (*resetfn)(struct VFIOPCIDevice *);
 uint32_t features;
 #define VFIO_FEATURE_ENABLE_VGA_BIT 0
@@ -171,6 +172,7 @@ typedef struct VFIOPCIDevice {
 bool req_enabled;
 bool has_flr;
 bool has_pm_reset;
+bool needs_bus_reset;
 bool rom_read_failed;
 } VFIOPCIDevice;
 
@@ -2977,6 +2979,81 @@ static int vfio_check_devices_host_bus_reset(void)
 return 0;
 }
 
+/* Update all the reset state of the affected devices in VM */
+static void vfio_pci_host_needs_bus_reset(Notifier *n, void *opaque)
+{
+VFIOPCIDevice *vdev = container_of(n, VFIOPCIDevice, 
sec_bus_reset_notifier);
+VFIODevice *vbasedev = vdev-vbasedev;
+PCIDevice *parent = opaque;
+VFIOGroup *group;
+struct vfio_pci_hot_reset_info *info = NULL;
+struct vfio_pci_dependent_device *devices;
+int i, ret;
+
+ret = vfio_get_hot_reset_info(vdev, info);
+if (ret) {
+goto out;
+}
+
+vdev-needs_bus_reset = true;
+/*
+ * qbus_reset_all always reset the devices from the depth level,
+ * here only need to do the reset for the device encounter the aer,
+ * in order to avoid reset the affected device first below the bus,
+ * we only set the top level devices as needs_reset.
+ */
+if (parent == pci_bridge_get_device(vdev-pdev.bus)) {
+vbasedev-needs_reset = true;
+}
+
+/* List all affected devices by bus reset */
+devices = info-devices[0];
+
+/* Verify that we have all the groups required */
+for (i = 0; i  info-count; i++) {
+PCIHostDeviceAddress host;
+VFIOPCIDevice *tmp;
+VFIODevice *vbasedev_iter;
+
+host.domain = devices[i].segment;
+host.bus = devices[i].bus;
+host.slot = PCI_SLOT(devices[i].devfn);
+host.function = PCI_FUNC(devices[i].devfn);
+
+/* Skip the current device */
+if (vfio_pci_host_match(host, vdev-host)) {
+continue;
+}
+
+/* Ensure we own the group of the affected device */
+QLIST_FOREACH(group, vfio_group_list, next) {
+if (group-groupid == devices[i].group_id) {
+break;
+}
+}
+
+if (!group) {
+goto out;
+}
+
+/* update all affected device the bus reset status */
+QLIST_FOREACH(vbasedev_iter, group-device_list, next) {
+if (vbasedev_iter-type != VFIO_DEVICE_TYPE_PCI) {
+continue;
+}
+tmp = container_of(vbasedev_iter, VFIOPCIDevice, vbasedev);
+if (vfio_pci_host_match(host, tmp-host)) {
+tmp-needs_bus_reset = true;
+tmp-vbasedev.needs_reset = false;
+break;
+}
+}
+}
+
+out:
+g_free(info);
+}
+
 static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t cap_ver,
   int pos, uint16_t size)
 {
@@ -3045,6 +3122,9 @@ static int vfio_setup_aer(VFIOPCIDevice *vdev, uint8_t 
cap_ver,
pdev-exp.aer_cap + PCI_ERR_UNCOR_SEVER, 4);
 pci_long_test_and_clear_mask(exp_cap + PCI_ERR_UNCOR_SEVER, ~severity);
 
+vdev-sec_bus_reset_notifier.notify = vfio_pci_host_needs_bus_reset;
+pci_bus_add_reset_notifier(pdev-bus, vdev-sec_bus_reset_notifier);
+
 return 0;
 
 error:
@@ -4029,6 +4109,9 @@ static void vfio_exitfn(PCIDevice *pdev)
 
 vfio_unregister_req_notifier(vdev);
 vfio_unregister_err_notifier(vdev);
+if (vdev-features  VFIO_FEATURE_ENABLE_AER) {
+pci_bus_remove_reset_notifier(vdev-sec_bus_reset_notifier);
+}
 pci_device_set_intx_routing_notifier(vdev-pdev, NULL);
 vfio_disable_interrupts(vdev);
 if (vdev-intx.mmap_timer) {
-- 
2.1.0




[Qemu-devel] [PATCH v12 06/15] aer: impove pcie_aer_init to support vfio device

2015-07-15 Thread Chen Hanxiao
From: Chen Fan chen.fan.f...@cn.fujitsu.com

pcie_aer_init was used to emulate an aer capability for pcie device,
but for vfio device, the aer config space size is mutable and is not
always equal to PCI_ERR_SIZEOF(0x48). it depends on where the TLP Prefix
register required, so here we add a size argument.

Signed-off-by: Chen Fan chen.fan.f...@cn.fujitsu.com
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/pci-bridge/ioh3420.c| 2 +-
 hw/pci-bridge/xio3130_downstream.c | 2 +-
 hw/pci-bridge/xio3130_upstream.c   | 2 +-
 hw/pci/pcie_aer.c  | 4 ++--
 include/hw/pci/pcie_aer.h  | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index cce2fdd..4d9cd3f 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -129,7 +129,7 @@ static int ioh3420_initfn(PCIDevice *d)
 goto err_pcie_cap;
 }
 pcie_cap_root_init(d);
-rc = pcie_aer_init(d, IOH_EP_AER_OFFSET);
+rc = pcie_aer_init(d, IOH_EP_AER_OFFSET, PCI_ERR_SIZEOF);
 if (rc  0) {
 goto err;
 }
diff --git a/hw/pci-bridge/xio3130_downstream.c 
b/hw/pci-bridge/xio3130_downstream.c
index b3a6479..9737041 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -92,7 +92,7 @@ static int xio3130_downstream_initfn(PCIDevice *d)
 goto err_pcie_cap;
 }
 pcie_cap_arifwd_init(d);
-rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
+rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
 if (rc  0) {
 goto err;
 }
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index eada582..4d7f894 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -81,7 +81,7 @@ static int xio3130_upstream_initfn(PCIDevice *d)
 }
 pcie_cap_flr_init(d);
 pcie_cap_deverr_init(d);
-rc = pcie_aer_init(d, XIO3130_AER_OFFSET);
+rc = pcie_aer_init(d, XIO3130_AER_OFFSET, PCI_ERR_SIZEOF);
 if (rc  0) {
 goto err;
 }
diff --git a/hw/pci/pcie_aer.c b/hw/pci/pcie_aer.c
index f1847ac..786f25d 100644
--- a/hw/pci/pcie_aer.c
+++ b/hw/pci/pcie_aer.c
@@ -94,12 +94,12 @@ static void aer_log_clear_all_err(PCIEAERLog *aer_log)
 aer_log-log_num = 0;
 }
 
-int pcie_aer_init(PCIDevice *dev, uint16_t offset)
+int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size)
 {
 PCIExpressDevice *exp;
 
 pcie_add_capability(dev, PCI_EXT_CAP_ID_ERR, PCI_ERR_VER,
-offset, PCI_ERR_SIZEOF);
+offset, size);
 exp = dev-exp;
 exp-aer_cap = offset;
 
diff --git a/include/hw/pci/pcie_aer.h b/include/hw/pci/pcie_aer.h
index 2fb8388..156acb0 100644
--- a/include/hw/pci/pcie_aer.h
+++ b/include/hw/pci/pcie_aer.h
@@ -87,7 +87,7 @@ struct PCIEAERErr {
 
 extern const VMStateDescription vmstate_pcie_aer_log;
 
-int pcie_aer_init(PCIDevice *dev, uint16_t offset);
+int pcie_aer_init(PCIDevice *dev, uint16_t offset, uint16_t size);
 void pcie_aer_exit(PCIDevice *dev);
 void pcie_aer_write_config(PCIDevice *dev,
uint32_t addr, uint32_t val, int len);
-- 
2.1.0




[Qemu-devel] [PATCH v2] pci_add_capability: remove duplicate comments

2015-07-14 Thread Chen Hanxiao
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/pci/pci.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 442f822..a017614 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2101,12 +2101,10 @@ static void pci_del_option_rom(PCIDevice *pdev)
 }
 
 /*
- * if !offset
- * Reserve space and add capability to the linked list in pci config space
- *
  * if offset = 0,
  * Find and reserve space and add capability to the linked list
- * in pci config space */
+ * in pci config space
+ */
 int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size)
 {
-- 
2.1.0




[Qemu-devel] [PATCH] pci_add_capability: remove duplicate comments

2015-07-13 Thread Chen Hanxiao
Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 hw/pci/pci.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 442f822..dfd2abc 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2101,12 +2101,10 @@ static void pci_del_option_rom(PCIDevice *pdev)
 }
 
 /*
- * if !offset
- * Reserve space and add capability to the linked list in pci config space
- *
  * if offset = 0,
  * Find and reserve space and add capability to the linked list
- * in pci config space */
+ * in pci config space
+ * /
 int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size)
 {
-- 
2.1.0




[Qemu-devel] [PATCH] docs/writing-qmp-commands: fix a typo

2015-05-29 Thread Chen Hanxiao
s/interation/iteration

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 docs/writing-qmp-commands.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/writing-qmp-commands.txt b/docs/writing-qmp-commands.txt
index f3df206..ab1fdd3 100644
--- a/docs/writing-qmp-commands.txt
+++ b/docs/writing-qmp-commands.txt
@@ -598,7 +598,7 @@ stored in its value member. In our example, the value 
member is a pointer
 to an TimerAlarmMethod instance.
 
 Notice that the current variable is used as true only in the first
-interation of the loop. That's because the alarm timer method in use is the
+iteration of the loop. That's because the alarm timer method in use is the
 first element of the alarm_timers array. Also notice that QAPI lists are 
handled
 by hand and we return the head of the list.
 
-- 
2.1.0




Re: [Qemu-devel] [PATCH] Use SCSI command to get size of SG device

2012-10-10 Thread Chen HanXiao
 -Original Message-
 From: Paolo Bonzini [mailto:paolo.bonz...@gmail.com] On Behalf Of Paolo
 Bonzini
 Sent: Wednesday, October 10, 2012 2:34 PM
 To: Chen HanXiao
 Cc: 'Christoph Hellwig'; qemu-devel@nongnu.org
 Subject: Re: [PATCH] Use SCSI command to get size of SG device
 
 Il 10/10/2012 04:11, Chen HanXiao ha scritto:
  
   On Tue, Oct 09, 2012 at 05:42:01PM +0800, Chen HanXiao wrote:
When we use SCSI generic device as disk image, function lseek
could not get the size of this kind of device.
So try to use SCSI command Read Capacity(10) when lseek failed
to get the size of SCSI generic device.
  
   Eww, this is ugly as hell.  Why would you even need the size for a
   raw passthrough device?
 
  If we want to enable snapshot for SCSI generic device as disk image,
  the size of SCSI generic device is needed. Function lseek could not
  get this, SCSI command can finish the job.
  Only when lseek failed would Read Capacity command be sent.
 
 You need to use scsi-block instead of scsi-generic.  However, I don't see
how
 this can work.  After the snapshot, the image will be qcow2, not raw, and
thus
 it will not support bdrv_aio_ioctl.  Hence any SCSI command (for
scsi-generic)
 or any non-data SCSI command (for scsi-block) will fail.
 
That's the issue what I also encountered.
Do you mean that it is impossible for us to enable snapshot for
scsi-generic?
Or patched for qcow2 would solve this?
 Can you give an example of what exactly you are trying to do?
 
I could enable snapshot for scsi-block
device, but failed when using scsi-generic with parameter 'snapshot =on'. 
The first issue is failing to get the size of SG device. So I tried to fix
it.

Command line:
-drive if=none,id=hd,file=/dev/sg2,snapshot=on \
-device virtio-scsi-pci,id=scsi --enable-kvm \
-device scsi-generic,drive=hd,id=vd1
 Paolo

Regards





[Qemu-devel] [PATCH] Use SCSI command to get size of SG device

2012-10-09 Thread Chen HanXiao
When we use SCSI generic device as disk image, function lseek
could not get the size of this kind of device.
So try to use SCSI command Read Capacity(10) when lseek failed to get
the size of SCSI generic device.

Signed-off-by: Chen Hanxiao chenhanx...@cn.fujitsu.com
---
 block/raw-posix.c |   46 --
 1 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index 28d439f..0be7db8 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -53,6 +53,7 @@
 #include linux/cdrom.h
 #include linux/fd.h
 #include linux/fs.h
+#include scsi/sg.h
 #endif
 #ifdef CONFIG_FIEMAP
 #include linux/fiemap.h
@@ -147,6 +148,7 @@ typedef struct BDRVRawReopenState {
 } BDRVRawReopenState;
 
 static int fd_open(BlockDriverState *bs);
+static int64_t raw_getlength_ioctl(int fd);
 static int64_t raw_getlength(BlockDriverState *bs);
 
 #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__)
@@ -653,13 +655,53 @@ static int64_t raw_getlength(BlockDriverState *bs)
 {
 BDRVRawState *s = bs-opaque;
 int ret;
-
+int64_t len;
+
 ret = fd_open(bs);
 if (ret  0) {
 return ret;
 }
 
-return lseek(s-fd, 0, SEEK_END);
+len = lseek(s-fd, 0, SEEK_END);
+if ( len = 0) {
+return len;
+} else {
+len = raw_getlength_ioctl(s-fd);
+return len;
+}
+}
+
+/* Use SCSI Read Capacity(10) Command to get length */
+static int64_t raw_getlength_ioctl(int fd)
+{
+unsigned char CDB[10] =
+{0x25, 0, 0, 0, 0, 0, 0, 0, 0, 0};
+unsigned char sense_buffer[32];
+unsigned char resp_buffer[32];
+uint32_t block_size;
+uint64_t last_blk_addr;
+struct sg_io_hdr io_hdr;
+int64_t ret;
+ 
+memset(io_hdr, 0, sizeof(struct sg_io_hdr));
+memset(sense_buffer, 0, sizeof(sense_buffer));
+memset(sense_buffer, 0, sizeof(resp_buffer));
+io_hdr.interface_id = 'S';
+io_hdr.dxfer_direction = SG_DXFER_FROM_DEV;
+io_hdr.cmd_len = sizeof(CDB);
+io_hdr.cmdp = CDB;
+io_hdr.sbp = sense_buffer;
+io_hdr.dxferp = resp_buffer;
+io_hdr.dxfer_len = sizeof(resp_buffer);
+if((ret = ioctl(fd, SG_IO, io_hdr))  0)
+return ret;
+
+last_blk_addr = ((resp_buffer[0]  24) | (resp_buffer[1]  16) |
+(resp_buffer[2]  8) | resp_buffer[3]);
+block_size = ((resp_buffer[4]  24) | (resp_buffer[5]  16) |
+(resp_buffer[6]  8) | resp_buffer[7]);
+ret = (int64_t)((last_blk_addr + 1) * block_size);
+return ret;
 }
 #endif
 
-- 
1.7.1






Re: [Qemu-devel] [PATCH] Use SCSI command to get size of SG device

2012-10-09 Thread Chen HanXiao
Hi
 -Original Message-
 From: Christoph Hellwig [mailto:h...@lst.de]
 Sent: Wednesday, October 10, 2012 1:21 AM
 To: Chen HanXiao
 Cc: qemu-devel@nongnu.org
 Subject: Re: [Qemu-devel] [PATCH] Use SCSI command to get size of SG
device
 
 On Tue, Oct 09, 2012 at 05:42:01PM +0800, Chen HanXiao wrote:
  When we use SCSI generic device as disk image, function lseek could
  not get the size of this kind of device.
  So try to use SCSI command Read Capacity(10) when lseek failed to get
  the size of SCSI generic device.
 
 Eww, this is ugly as hell.  Why would you even need the size for a raw
 passthrough device?

If we want to enable snapshot for SCSI generic device as disk image, the
size of 
SCSI generic device is needed. Function lseek could not get this, SCSI
command 
can finish the job.
Only when lseek failed would Read Capacity command be sent. 

Regards