Vinzenz Feenstra has posted comments on this change.

Change subject: virt: Only filter guest agent data, only on XMLRPC
......................................................................


Patch Set 17:

(2 comments)

@nsoffer:
I personally do not have the profiling data. It was brought to my attention 
during the investigation in here:
https://bugzilla.redhat.com/show_bug.cgi?id=1177634#c53

As part of that, we did some optimizations on the side of the filtering, if you 
recall. However we concluded due to the nature of JSON escaping strings, this 
filtering process is something which is not necessary to be applied on JSON RPC.

This filtering was required to keep XML documents valid, however it's resource 
intensive to execute therefore we wanted to apply the filtering now only IF we 
have XMLRPC to not break the validity of the XML documents on XMLRPC.

https://gerrit.ovirt.org/#/c/36949/17//COMMIT_MSG
Commit Message:

Line 3: AuthorDate: 2015-01-07 11:32:48 +0100
Line 4: Commit:     Vinzenz Feenstra <[email protected]>
Line 5: CommitDate: 2015-07-13 09:19:26 +0200
Line 6: 
Line 7: virt: Only filter guest agent data, only on XMLRPC
> would you add a word about the motivation for this? (reducing CPU consumpti
all funny characters are getting escaped in JSON so jsonrpc itself won't be 
affected. The problem we have had earlier with XMLRPC was that there were 
values in UTF-8 which were not allowed in XML this is not the case for JSON, as 
the json strings which would cause those issues are escaped.

The point is, that if we would have this issue with JSON that problem would 
have to already be present at the time we're getting the data in VDSM from the 
guest agent.
Line 8: 
Line 9: As a side effect of this change, the data has to be all unicode
Line 10: in order to have the filtering applied properly.
Line 11: 


Line 6: 
Line 7: virt: Only filter guest agent data, only on XMLRPC
Line 8: 
Line 9: As a side effect of this change, the data has to be all unicode
Line 10: in order to have the filtering applied properly.
> sorry, I don't understand why was that required, or why all string literals
That's not true anymore I will have to remove this part of the commit message.
Line 11: 
Line 12: Change-Id: Ia9ee2a8f1cc6784c619ce68da3cb9342b0d72cfc
Line 13: Bug-Url: https://bugzilla.redhat.com/1179696


-- 
To view, visit https://gerrit.ovirt.org/36949
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia9ee2a8f1cc6784c619ce68da3cb9342b0d72cfc
Gerrit-PatchSet: 17
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Michal Skrivanek <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Vinzenz Feenstra <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to