Patchwork [12/22] qom-fuse: workaround for truncated properties > 4096

login
register
mail settings
Submitter Michael Roth
Date July 24, 2012, 5:20 p.m.
Message ID <1343150454-4677-13-git-send-email-mdroth@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/172988/
State New
Headers show

Comments

Michael Roth - July 24, 2012, 5:20 p.m.
We currently hard-code property size at 4096 for the purposes of
getattr()/stat()/etc. For 'state' properties we can exceed this easily,
leading to truncated responses.

Instead, for a particular property, make it
max(4096, most_recent_property_size * 2). This allows some
head-room for properties that change size periodically (numbers,
strings, state properties containing arrays, etc)

Also, implement a simple property cache to avoid spinning on qom-get
if an application reads beyond the actual size. This also allows us
to use a snapshot of a single qom-get that persists across read()'s.
Old Cache entries are evicted as soon as we attempt to read() from
offset 0 again.

Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 QMP/qom-fuse |   24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)
Anthony Liguori - July 24, 2012, 10:26 p.m.
Michael Roth <mdroth@linux.vnet.ibm.com> writes:

I don't think this is the most correct solution but I think it's good
enough in practice so

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

Regards,

Anthony Liguori

> We currently hard-code property size at 4096 for the purposes of
> getattr()/stat()/etc. For 'state' properties we can exceed this easily,
> leading to truncated responses.
>
> Instead, for a particular property, make it
> max(4096, most_recent_property_size * 2). This allows some
> head-room for properties that change size periodically (numbers,
> strings, state properties containing arrays, etc)
>
> Also, implement a simple property cache to avoid spinning on qom-get
> if an application reads beyond the actual size. This also allows us
> to use a snapshot of a single qom-get that persists across read()'s.
> Old Cache entries are evicted as soon as we attempt to read() from
> offset 0 again.
>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  QMP/qom-fuse |   24 +++++++++++++++++-------
>  1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/QMP/qom-fuse b/QMP/qom-fuse
> index 5c6754a..bd43f29 100755
> --- a/QMP/qom-fuse
> +++ b/QMP/qom-fuse
> @@ -26,6 +26,7 @@ class QOMFS(Fuse):
>          self.qmp.connect()
>          self.ino_map = {}
>          self.ino_count = 1
> +        self.prop_cache = {}
>  
>      def get_ino(self, path):
>          if self.ino_map.has_key(path):
> @@ -67,12 +68,16 @@ class QOMFS(Fuse):
>          if not self.is_property(path):
>              return -ENOENT
>  
> -        path, prop = path.rsplit('/', 1)
> -        try:
> -            data = str(self.qmp.command('qom-get', path=path, property=prop))
> -            data += '\n' # make values shell friendly
> -        except:
> -            return -EPERM
> +        # avoid extra calls to qom-get by using cached value when offset > 0
> +        if offset == 0 or not self.prop_cache.has_key(path):
> +            directory, prop = path.rsplit('/', 1)
> +            try:
> +                resp = str(self.qmp.command('qom-get', path=directory, property=prop))
> +                self.prop_cache[path] = resp + '\n' # make values shell friendly
> +            except:
> +                return -EPERM
> +
> +        data = self.prop_cache[path]
>  
>          if offset > len(data):
>              return ''
> @@ -111,13 +116,18 @@ class QOMFS(Fuse):
>                                         0,
>                                         0))
>          elif self.is_property(path):
> +            directory, prop = path.rsplit('/', 1)
> +            try:
> +                resp = str(self.qmp.command('qom-get', path=directory, property=prop))
> +            except:
> +                return -ENOENT
>              value = posix.stat_result((0644 | stat.S_IFREG,
>                                         self.get_ino(path),
>                                         0,
>                                         1,
>                                         1000,
>                                         1000,
> -                                       4096,
> +                                       max(len(resp) * 2, 4096),
>                                         0,
>                                         0,
>                                         0))
> -- 
> 1.7.9.5

Patch

diff --git a/QMP/qom-fuse b/QMP/qom-fuse
index 5c6754a..bd43f29 100755
--- a/QMP/qom-fuse
+++ b/QMP/qom-fuse
@@ -26,6 +26,7 @@  class QOMFS(Fuse):
         self.qmp.connect()
         self.ino_map = {}
         self.ino_count = 1
+        self.prop_cache = {}
 
     def get_ino(self, path):
         if self.ino_map.has_key(path):
@@ -67,12 +68,16 @@  class QOMFS(Fuse):
         if not self.is_property(path):
             return -ENOENT
 
-        path, prop = path.rsplit('/', 1)
-        try:
-            data = str(self.qmp.command('qom-get', path=path, property=prop))
-            data += '\n' # make values shell friendly
-        except:
-            return -EPERM
+        # avoid extra calls to qom-get by using cached value when offset > 0
+        if offset == 0 or not self.prop_cache.has_key(path):
+            directory, prop = path.rsplit('/', 1)
+            try:
+                resp = str(self.qmp.command('qom-get', path=directory, property=prop))
+                self.prop_cache[path] = resp + '\n' # make values shell friendly
+            except:
+                return -EPERM
+
+        data = self.prop_cache[path]
 
         if offset > len(data):
             return ''
@@ -111,13 +116,18 @@  class QOMFS(Fuse):
                                        0,
                                        0))
         elif self.is_property(path):
+            directory, prop = path.rsplit('/', 1)
+            try:
+                resp = str(self.qmp.command('qom-get', path=directory, property=prop))
+            except:
+                return -ENOENT
             value = posix.stat_result((0644 | stat.S_IFREG,
                                        self.get_ino(path),
                                        0,
                                        1,
                                        1000,
                                        1000,
-                                       4096,
+                                       max(len(resp) * 2, 4096),
                                        0,
                                        0,
                                        0))