Hi, Marc-André Thanks for reply
2012/11/23 Marc-André Lureau <[email protected]>: > Hi > > ----- Mensaje original ----- >> At present, Vmware and Virtualbox has supported file drag&drop >> feature, >> I think it's a good feature for users, so we want qemu/spice to >> supports it. > > Btw, have you done a small comparison of what VirtualBox can support and how > it is implemented? I made a simple research how virtualbox implement dnd on client side, since only client source is opensource. It's more complex than what we do. You can take a simple look at in this page: http://www.virtualbox.org/svn/vbox/trunk/include/VBox/HostServices/DragAndDropSvc.h > >> This patch first adds communication protocol between client and >> guest, >> we must make the agent protocol stable before coding, this is what we >> want this patch to do. >> >> This feature has been discussed on spice mailing list. >> >> The more details are available at following pages: >> http://lists.freedesktop.org/archives/spice-devel/2012-November/011400.html >> and >> http://lists.freedesktop.org/archives/spice-devel/2012-November/011485.html >> >> Signed-off-by: Dunrong Huang <[email protected]> >> Cc: Hans de Goede <[email protected]> >> Cc: Marc-André Lureau <[email protected]> >> Cc: Alon Levy <[email protected]> >> Cc: Uri Lublin <[email protected]> >> --- >> spice/vd_agent.h | 35 +++++++++++++++++++++++++++++++++++ >> 1 file changed, 35 insertions(+) >> >> diff --git a/spice/vd_agent.h b/spice/vd_agent.h >> index 2b9884e..f6cc49e 100644 >> --- a/spice/vd_agent.h >> +++ b/spice/vd_agent.h >> @@ -69,9 +69,44 @@ enum { >> VD_AGENT_CLIPBOARD_GRAB, >> VD_AGENT_CLIPBOARD_REQUEST, >> VD_AGENT_CLIPBOARD_RELEASE, >> + VD_AGENT_FILE_XFER_START, >> + VD_AGENT_FILE_XFER_STATUS, >> + VD_AGENT_FILE_XFER_DATA, >> VD_AGENT_END_MESSAGE, >> }; >> >> +enum { >> + VD_AGENT_FILE_XFER_RESULT_SUCCESS, > > What does success indicates? When is it sent? > >> + VD_AGENT_FILE_XFER_RESULT_DISK_FULL, > > What can the client do about it that the guest can't? I am not sure we need > separate error for "disk full" situation. If we do, then I guess we need a > lot more errors (invalid filename, file too long, exists, permission denied, > read only...) > >> + VD_AGENT_FILE_XFER_RESULT_CANCELLED, > > In addition (can be added later on), I think we could use PAUSE/RESUME. > >> + VD_AGENT_FILE_XFER_RESULT_ERROR, >> +}; > > Rather than "result", it looks like VD_AGENT_FILE_XFER_STATUS_ would fit > better, since it is embedded the StatusMessage. > >> + >> +typedef struct SPICE_ATTR_PACKED VDAgentFileXferStatusMessage { >> + uint32_t id; >> + uint32_t result; >> +} VDAgentFileXferStatusMessage; >> + >> +enum { >> + VD_AGENT_FILE_XFER_FORMAT_RAW, >> + VD_AGENT_FILE_XFER_RESULT_GZIP, >> +}; > > enum prefix XFER_FORMAT != XFER_RESULT Oop, It's a typo. > > If we have to include a archive/compression I would go for tar.xz directly. I > think compression could be added later. Actually, compressed_format message has not been used in current code, I defined it for future work here. But you are right, it should be added later. > >> + >> +typedef struct SPICE_ATTR_PACKED VDAgentFileXferStartMessage { >> + uint32_t id; >> + uint32_t is_dir; > > What if is_dir is true? How do you know each file name & size then? Or you > send a separate message just for the directory creation? If is_dir is true, just create the directory. For example: A dir named "doc" has two files a.txt and b.txt. First we send a start message with file_name = "doc" then message with file_name = "doc/a.txt" and message with file_name = "doc/b.txt" will be sent. So guest can create a direcory doc, and doc/a.txt, doc/b.txt. In this way, we can implement transfer directory. > >> + uint32_t compressed_format; >> + uint32_t file_mode; >> + uint64_t file_size; > > Just some thoughts: > If the file is compressed it will be hard to know in advance the file_size, > unless we compress all before sending, which is a waste of resources. > file_size could be the final decompress size. And after sending all the > VDAgentFileXferDataMessage, the client could send a final > VD_AGENT_FILE_XFER_RESULT_SUCCESS, to indicate eof? > >> + uint8_t file_name[0]; >> +} VDAgentFileXferStartMessage; > > Instead of those fields that will be hard to extend and that mostly only > concern the guest agent, I would consider a key-value text such as: > > [vdagent-file-xfer] > type=regular > mime=text/plain > name=foo.sh > time-modified=1353085649 > time-... > can-read=true > can-execute=true > can-write=false > size=25 > > [vdagent-file-xfer] > type=directory > mime=application/x-xz-compressed-tar > name=Downloads > time-modified=1353085649 > time-... > can-read=true > can-execute=true > can-write=false > size=515416 > [vdagent-context] > pointer-position=+34+98 > > We need to define the mandatory group (vdagent-file-xfer) and keys (file, > type, mime, size..). > > If qemu needs to apply some kind of filtering (which I think is really hard > to do, you basically allow copy or not at all), some of the fields could be > in the message itself eventually, to avoid qemu parsing that text file. > >> +typedef struct SPICE_ATTR_PACKED VDAgentFileXferDataMessage { >> + uint32_t id; >> + uint64_t size; > > uint64_t is certainly too large, and you can deduce data size from the > message size. > >> + uint8_t data[0]; >> +} VDAgentFileXferDataMessage; >> + > > Data can be sent right away without waiting for status message? In which > case, the client will need to queue or save in temp the received data, but > why not. > > Thanks again for your work! -- Best Regards, Dunrong Huang _______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
